[alsa-devel] [PATCH v2 0/2] ASoC: omap-mcpdm: New McPDM driver
Hello Mark, Liam,
I tried to address the comments regarding to version 1 of the new McPDM driver: http://mailman.alsa-project.org/pipermail/alsa-devel/2011-July/041690.html
The omap hwmod conversion part of the v1 series has been sent as part of the fixup series for 3.1 to make basic audio work out of box.
This series is aimed for 3.2 due to the scale of the change. I tried to split patch 2, but I did not managed to find an incremental path from the old driver to the new one.
I have updated the commit message for patch 2 to explain the reasoning behind the rewritten driver, so it can be easily looked up.
Notable changes since v1: - delayed_work from the new driver has been removed, and replaced with a DAPM_SUPPLY widget. This widget need to be connected to DAC/ADC of the codec by the machine driver in order to have the correct sequence for power off. The first patch adds private_data pointer for dapm widgets. This private_data can be used to add widget specific data to any widget. I think this can be handy for others as well in the future.
- Full duplex audio in legacy mode has been fixed, so we can run arecord | aplay
This series depends on the McPDM fixup patches: http://mailman.alsa-project.org/pipermail/alsa-devel/2011-August/042401.html
Peter
PS: to make the review easier, I have asked git format-patch to treat the change as rewrite.
--- Misael Lopez Cruz (1): ASoC: omap-mcpdm: Replace legacy driver
Peter Ujfalusi (1): ASoC: DAPM: Add private data pointer for DAPM widget
include/sound/soc-dapm.h | 1 + sound/soc/omap/Makefile | 2 +- sound/soc/omap/mcpdm.c | 472 ------------------------- sound/soc/omap/mcpdm.h | 152 -------- sound/soc/omap/omap-mcpdm.c | 823 ++++++++++++++++++++++++++++--------------- sound/soc/omap/omap-mcpdm.h | 97 +++++ sound/soc/omap/sdp4430.c | 14 +- 7 files changed, 656 insertions(+), 905 deletions(-) delete mode 100644 sound/soc/omap/mcpdm.c delete mode 100644 sound/soc/omap/mcpdm.h rewrite sound/soc/omap/omap-mcpdm.c (46%) create mode 100644 sound/soc/omap/omap-mcpdm.h
Support for using custom private data assigned per DAPM widget.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- include/sound/soc-dapm.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h index 602024d..84114e7 100644 --- a/include/sound/soc-dapm.h +++ b/include/sound/soc-dapm.h @@ -467,6 +467,7 @@ struct snd_soc_dapm_widget { unsigned char force:1; /* force state */ unsigned char ignore_suspend:1; /* kept enabled over suspend */ int subseq; /* sort within widget type */ + void *private_data;
int (*power_check)(struct snd_soc_dapm_widget *w);
On Fri, Aug 19, 2011 at 10:41:18AM +0300, Peter Ujfalusi wrote:
Support for using custom private data assigned per DAPM widget.
If we're adding this we should probably go the accessor route like we do with the driver data for the CODECs.
On Saturday 20 August 2011 09:02:20 Mark Brown wrote:
On Fri, Aug 19, 2011 at 10:41:18AM +0300, Peter Ujfalusi wrote:
Support for using custom private data assigned per DAPM widget.
If we're adding this we should probably go the accessor route like we do with the driver data for the CODECs.
Something like:
static inline void *snd_soc_widget_get_pdata(struct snd_soc_dapm_widget *w) { return w->private_data; }
static inline void snd_soc_widget_set_pdata(struct snd_soc_dapm_widget *w, void *data) { w->private_data = data; }
Another option at the moment is that I create global variable within the omap- mcpdm.c, so I can have access to the mcpdm within the dapm event handler.
On Mon, Aug 22, 2011 at 04:33:20PM +0300, Péter Ujfalusi wrote:
On Saturday 20 August 2011 09:02:20 Mark Brown wrote:
If we're adding this we should probably go the accessor route like we do with the driver data for the CODECs.
Something like:
static inline void *snd_soc_widget_get_pdata(struct snd_soc_dapm_widget *w) { return w->private_data; }
Yeah, something like that.
Another option at the moment is that I create global variable within the omap- mcpdm.c, so I can have access to the mcpdm within the dapm event handler.
That'll probably break in future I'd guess.
From: Misael Lopez Cruz misael.lopez@ti.com
Reasons for the replacement: The current driver for McPDM was developed to support the legacy mode only. In preparation for the ABE support the current driver stack need the be replaced. The new driver is much simpler, easier to extend, and it also fixes some of the issues with the old stack.
Main changes: - single file for omap-mcpdm (mcpdm.c/h removed) - Define names for registers, bits cleaned up, prefixed - Full-duplex audio operation (arecord | aplay) has been fixed - Less code
McPDM need to be turned off _after_ the codec's DAC/ADC has been powered down. The solution for this without extensive change in core: Use DAPM_SUPPLY to turn off the McPDM interface. The machine driver connects the "OMAP McPDM Interface" to the codec's DAC/ADC, so after audio activity we can be sure that the host side McPDM is turned off at the correct time.
Signed-off-by: Misael Lopez Cruz misael.lopez@ti.com Signed-off-by: Liam Girdwood lrg@ti.com Signed-off-by: Sebastien Guiriec s-guiriec@ti.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/Makefile | 2 +- sound/soc/omap/mcpdm.c | 472 ------------------------- sound/soc/omap/mcpdm.h | 152 -------- sound/soc/omap/omap-mcpdm.c | 823 ++++++++++++++++++++++++++++--------------- sound/soc/omap/omap-mcpdm.h | 97 +++++ sound/soc/omap/sdp4430.c | 14 +- 6 files changed, 655 insertions(+), 905 deletions(-) delete mode 100644 sound/soc/omap/mcpdm.c delete mode 100644 sound/soc/omap/mcpdm.h rewrite sound/soc/omap/omap-mcpdm.c (46%) create mode 100644 sound/soc/omap/omap-mcpdm.h
diff --git a/sound/soc/omap/Makefile b/sound/soc/omap/Makefile index 59e2c8d..052fd75 100644 --- a/sound/soc/omap/Makefile +++ b/sound/soc/omap/Makefile @@ -1,7 +1,7 @@ # OMAP Platform Support snd-soc-omap-objs := omap-pcm.o snd-soc-omap-mcbsp-objs := omap-mcbsp.o -snd-soc-omap-mcpdm-objs := omap-mcpdm.o mcpdm.o +snd-soc-omap-mcpdm-objs := omap-mcpdm.o snd-soc-omap-hdmi-objs := omap-hdmi.o
obj-$(CONFIG_SND_OMAP_SOC) += snd-soc-omap.o diff --git a/sound/soc/omap/mcpdm.c b/sound/soc/omap/mcpdm.c deleted file mode 100644 index 9746a49..0000000 --- a/sound/soc/omap/mcpdm.c +++ /dev/null @@ -1,472 +0,0 @@ -/* - * mcpdm.c -- McPDM interface driver - * - * Author: Jorge Eduardo Candelaria x0107209@ti.com - * Copyright (C) 2009 - Texas Instruments, Inc. - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * version 2 as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA - * 02110-1301 USA - * - */ - -#include <linux/module.h> -#include <linux/init.h> -#include <linux/device.h> -#include <linux/platform_device.h> -#include <linux/wait.h> -#include <linux/slab.h> -#include <linux/interrupt.h> -#include <linux/err.h> -#include <linux/pm_runtime.h> -#include <linux/delay.h> -#include <linux/io.h> -#include <linux/irq.h> - -#include "mcpdm.h" - -static struct omap_mcpdm *mcpdm; - -static inline void omap_mcpdm_write(u16 reg, u32 val) -{ - __raw_writel(val, mcpdm->io_base + reg); -} - -static inline int omap_mcpdm_read(u16 reg) -{ - return __raw_readl(mcpdm->io_base + reg); -} - -static void omap_mcpdm_reg_dump(void) -{ - dev_dbg(mcpdm->dev, "***********************\n"); - dev_dbg(mcpdm->dev, "IRQSTATUS_RAW: 0x%04x\n", - omap_mcpdm_read(MCPDM_IRQSTATUS_RAW)); - dev_dbg(mcpdm->dev, "IRQSTATUS: 0x%04x\n", - omap_mcpdm_read(MCPDM_IRQSTATUS)); - dev_dbg(mcpdm->dev, "IRQENABLE_SET: 0x%04x\n", - omap_mcpdm_read(MCPDM_IRQENABLE_SET)); - dev_dbg(mcpdm->dev, "IRQENABLE_CLR: 0x%04x\n", - omap_mcpdm_read(MCPDM_IRQENABLE_CLR)); - dev_dbg(mcpdm->dev, "IRQWAKE_EN: 0x%04x\n", - omap_mcpdm_read(MCPDM_IRQWAKE_EN)); - dev_dbg(mcpdm->dev, "DMAENABLE_SET: 0x%04x\n", - omap_mcpdm_read(MCPDM_DMAENABLE_SET)); - dev_dbg(mcpdm->dev, "DMAENABLE_CLR: 0x%04x\n", - omap_mcpdm_read(MCPDM_DMAENABLE_CLR)); - dev_dbg(mcpdm->dev, "DMAWAKEEN: 0x%04x\n", - omap_mcpdm_read(MCPDM_DMAWAKEEN)); - dev_dbg(mcpdm->dev, "CTRL: 0x%04x\n", - omap_mcpdm_read(MCPDM_CTRL)); - dev_dbg(mcpdm->dev, "DN_DATA: 0x%04x\n", - omap_mcpdm_read(MCPDM_DN_DATA)); - dev_dbg(mcpdm->dev, "UP_DATA: 0x%04x\n", - omap_mcpdm_read(MCPDM_UP_DATA)); - dev_dbg(mcpdm->dev, "FIFO_CTRL_DN: 0x%04x\n", - omap_mcpdm_read(MCPDM_FIFO_CTRL_DN)); - dev_dbg(mcpdm->dev, "FIFO_CTRL_UP: 0x%04x\n", - omap_mcpdm_read(MCPDM_FIFO_CTRL_UP)); - dev_dbg(mcpdm->dev, "DN_OFFSET: 0x%04x\n", - omap_mcpdm_read(MCPDM_DN_OFFSET)); - dev_dbg(mcpdm->dev, "***********************\n"); -} - -/* - * Takes the McPDM module in and out of reset state. - * Uplink and downlink can be reset individually. - */ -static void omap_mcpdm_reset_capture(int reset) -{ - int ctrl = omap_mcpdm_read(MCPDM_CTRL); - - if (reset) - ctrl |= SW_UP_RST; - else - ctrl &= ~SW_UP_RST; - - omap_mcpdm_write(MCPDM_CTRL, ctrl); -} - -static void omap_mcpdm_reset_playback(int reset) -{ - int ctrl = omap_mcpdm_read(MCPDM_CTRL); - - if (reset) - ctrl |= SW_DN_RST; - else - ctrl &= ~SW_DN_RST; - - omap_mcpdm_write(MCPDM_CTRL, ctrl); -} - -/* - * Enables the transfer through the PDM interface to/from the Phoenix - * codec by enabling the corresponding UP or DN channels. - */ -void omap_mcpdm_start(int stream) -{ - int ctrl = omap_mcpdm_read(MCPDM_CTRL); - - if (stream) - ctrl |= mcpdm->up_channels; - else - ctrl |= mcpdm->dn_channels; - - omap_mcpdm_write(MCPDM_CTRL, ctrl); -} - -/* - * Disables the transfer through the PDM interface to/from the Phoenix - * codec by disabling the corresponding UP or DN channels. - */ -void omap_mcpdm_stop(int stream) -{ - int ctrl = omap_mcpdm_read(MCPDM_CTRL); - - if (stream) - ctrl &= ~mcpdm->up_channels; - else - ctrl &= ~mcpdm->dn_channels; - - omap_mcpdm_write(MCPDM_CTRL, ctrl); -} - -/* - * Configures McPDM uplink for audio recording. - * This function should be called before omap_mcpdm_start. - */ -int omap_mcpdm_capture_open(struct omap_mcpdm_link *uplink) -{ - int irq_mask = 0; - int ctrl; - - if (!uplink) - return -EINVAL; - - mcpdm->uplink = uplink; - - /* Enable irq request generation */ - irq_mask |= uplink->irq_mask & MCPDM_UPLINK_IRQ_MASK; - omap_mcpdm_write(MCPDM_IRQENABLE_SET, irq_mask); - - /* Configure uplink threshold */ - if (uplink->threshold > UP_THRES_MAX) - uplink->threshold = UP_THRES_MAX; - - omap_mcpdm_write(MCPDM_FIFO_CTRL_UP, uplink->threshold); - - /* Configure DMA controller */ - omap_mcpdm_write(MCPDM_DMAENABLE_SET, DMA_UP_ENABLE); - - /* Set pdm out format */ - ctrl = omap_mcpdm_read(MCPDM_CTRL); - ctrl &= ~PDMOUTFORMAT; - ctrl |= uplink->format & PDMOUTFORMAT; - - /* Uplink channels */ - mcpdm->up_channels = uplink->channels & (PDM_UP_MASK | PDM_STATUS_MASK); - - omap_mcpdm_write(MCPDM_CTRL, ctrl); - - return 0; -} - -/* - * Configures McPDM downlink for audio playback. - * This function should be called before omap_mcpdm_start. - */ -int omap_mcpdm_playback_open(struct omap_mcpdm_link *downlink) -{ - int irq_mask = 0; - int ctrl; - - if (!downlink) - return -EINVAL; - - mcpdm->downlink = downlink; - - /* Enable irq request generation */ - irq_mask |= downlink->irq_mask & MCPDM_DOWNLINK_IRQ_MASK; - omap_mcpdm_write(MCPDM_IRQENABLE_SET, irq_mask); - - /* Configure uplink threshold */ - if (downlink->threshold > DN_THRES_MAX) - downlink->threshold = DN_THRES_MAX; - - omap_mcpdm_write(MCPDM_FIFO_CTRL_DN, downlink->threshold); - - /* Enable DMA request generation */ - omap_mcpdm_write(MCPDM_DMAENABLE_SET, DMA_DN_ENABLE); - - /* Set pdm out format */ - ctrl = omap_mcpdm_read(MCPDM_CTRL); - ctrl &= ~PDMOUTFORMAT; - ctrl |= downlink->format & PDMOUTFORMAT; - - /* Downlink channels */ - mcpdm->dn_channels = downlink->channels & (PDM_DN_MASK | PDM_CMD_MASK); - - omap_mcpdm_write(MCPDM_CTRL, ctrl); - - return 0; -} - -/* - * Cleans McPDM uplink configuration. - * This function should be called when the stream is closed. - */ -int omap_mcpdm_capture_close(struct omap_mcpdm_link *uplink) -{ - int irq_mask = 0; - - if (!uplink) - return -EINVAL; - - /* Disable irq request generation */ - irq_mask |= uplink->irq_mask & MCPDM_UPLINK_IRQ_MASK; - omap_mcpdm_write(MCPDM_IRQENABLE_CLR, irq_mask); - - /* Disable DMA request generation */ - omap_mcpdm_write(MCPDM_DMAENABLE_CLR, DMA_UP_ENABLE); - - /* Clear Downlink channels */ - mcpdm->up_channels = 0; - - mcpdm->uplink = NULL; - - return 0; -} - -/* - * Cleans McPDM downlink configuration. - * This function should be called when the stream is closed. - */ -int omap_mcpdm_playback_close(struct omap_mcpdm_link *downlink) -{ - int irq_mask = 0; - - if (!downlink) - return -EINVAL; - - /* Disable irq request generation */ - irq_mask |= downlink->irq_mask & MCPDM_DOWNLINK_IRQ_MASK; - omap_mcpdm_write(MCPDM_IRQENABLE_CLR, irq_mask); - - /* Disable DMA request generation */ - omap_mcpdm_write(MCPDM_DMAENABLE_CLR, DMA_DN_ENABLE); - - /* clear Downlink channels */ - mcpdm->dn_channels = 0; - - mcpdm->downlink = NULL; - - return 0; -} - -static irqreturn_t omap_mcpdm_irq_handler(int irq, void *dev_id) -{ - struct omap_mcpdm *mcpdm_irq = dev_id; - int irq_status; - - irq_status = omap_mcpdm_read(MCPDM_IRQSTATUS); - - /* Acknowledge irq event */ - omap_mcpdm_write(MCPDM_IRQSTATUS, irq_status); - - if (irq_status & MCPDM_DN_IRQ_FULL) { - dev_err(mcpdm_irq->dev, "DN FIFO error %x\n", irq_status); - omap_mcpdm_reset_playback(1); - omap_mcpdm_playback_open(mcpdm_irq->downlink); - omap_mcpdm_reset_playback(0); - } - - if (irq_status & MCPDM_DN_IRQ_EMPTY) { - dev_err(mcpdm_irq->dev, "DN FIFO error %x\n", irq_status); - omap_mcpdm_reset_playback(1); - omap_mcpdm_playback_open(mcpdm_irq->downlink); - omap_mcpdm_reset_playback(0); - } - - if (irq_status & MCPDM_DN_IRQ) - dev_dbg(mcpdm_irq->dev, "DN write request\n"); - - if (irq_status & MCPDM_UP_IRQ_FULL) { - dev_err(mcpdm_irq->dev, "UP FIFO error %x\n", irq_status); - omap_mcpdm_reset_capture(1); - omap_mcpdm_capture_open(mcpdm_irq->uplink); - omap_mcpdm_reset_capture(0); - } - - if (irq_status & MCPDM_UP_IRQ_EMPTY) { - dev_err(mcpdm_irq->dev, "UP FIFO error %x\n", irq_status); - omap_mcpdm_reset_capture(1); - omap_mcpdm_capture_open(mcpdm_irq->uplink); - omap_mcpdm_reset_capture(0); - } - - if (irq_status & MCPDM_UP_IRQ) - dev_dbg(mcpdm_irq->dev, "UP write request\n"); - - return IRQ_HANDLED; -} - -int omap_mcpdm_request(void) -{ - int ret; - - pm_runtime_get_sync(mcpdm->dev); - - spin_lock(&mcpdm->lock); - - if (!mcpdm->free) { - dev_err(mcpdm->dev, "McPDM interface is in use\n"); - spin_unlock(&mcpdm->lock); - ret = -EBUSY; - goto err; - } - mcpdm->free = 0; - - spin_unlock(&mcpdm->lock); - - /* Disable lines while request is ongoing */ - omap_mcpdm_write(MCPDM_CTRL, 0x00); - - ret = request_irq(mcpdm->irq, omap_mcpdm_irq_handler, - 0, "McPDM", (void *)mcpdm); - if (ret) { - dev_err(mcpdm->dev, "Request for McPDM IRQ failed\n"); - goto err; - } - - return 0; - -err: - mcpdm->free = 1; - pm_runtime_put_sync(mcpdm->dev); - return ret; -} - -void omap_mcpdm_free(void) -{ - spin_lock(&mcpdm->lock); - if (mcpdm->free) { - dev_err(mcpdm->dev, "McPDM interface is already free\n"); - spin_unlock(&mcpdm->lock); - return; - } - mcpdm->free = 1; - spin_unlock(&mcpdm->lock); - - pm_runtime_put_sync(mcpdm->dev); - - free_irq(mcpdm->irq, (void *)mcpdm); -} - -/* Enable/disable DC offset cancelation for the analog - * headset path (PDM channels 1 and 2). - */ -int omap_mcpdm_set_offset(int offset1, int offset2) -{ - int offset; - - if ((offset1 > DN_OFST_MAX) || (offset2 > DN_OFST_MAX)) - return -EINVAL; - - offset = (offset1 << DN_OFST_RX1) | (offset2 << DN_OFST_RX2); - - /* offset cancellation for channel 1 */ - if (offset1) - offset |= DN_OFST_RX1_EN; - else - offset &= ~DN_OFST_RX1_EN; - - /* offset cancellation for channel 2 */ - if (offset2) - offset |= DN_OFST_RX2_EN; - else - offset &= ~DN_OFST_RX2_EN; - - omap_mcpdm_write(MCPDM_DN_OFFSET, offset); - - return 0; -} - -int __devinit omap_mcpdm_probe(struct platform_device *pdev) -{ - struct resource *res; - int ret = 0; - - mcpdm = kzalloc(sizeof(struct omap_mcpdm), GFP_KERNEL); - if (!mcpdm) { - ret = -ENOMEM; - goto exit; - } - - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (res == NULL) { - dev_err(&pdev->dev, "no resource\n"); - goto err_resource; - } - - spin_lock_init(&mcpdm->lock); - mcpdm->free = 1; - - if (!request_mem_region(res->start, resource_size(res), "McPDM")) { - ret = -EBUSY; - goto err_resource; - } - - mcpdm->io_base = ioremap(res->start, resource_size(res)); - if (!mcpdm->io_base) { - ret = -ENOMEM; - goto err_remap; - } - - mcpdm->irq = platform_get_irq(pdev, 0); - - mcpdm->dev = &pdev->dev; - platform_set_drvdata(pdev, mcpdm); - - pm_runtime_enable(mcpdm->dev); - - return 0; - -err_remap: - release_mem_region(res->start, resource_size(res)); -err_resource: - kfree(mcpdm); -exit: - return ret; -} - -int __devexit omap_mcpdm_remove(struct platform_device *pdev) -{ - struct omap_mcpdm *mcpdm_ptr = platform_get_drvdata(pdev); - struct resource *res; - - platform_set_drvdata(pdev, NULL); - - pm_runtime_disable(mcpdm_ptr->dev); - - iounmap(mcpdm_ptr->io_base); - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - release_mem_region(res->start, resource_size(res)); - - mcpdm_ptr->free = 0; - mcpdm_ptr->dev = NULL; - - kfree(mcpdm_ptr); - - return 0; -} - diff --git a/sound/soc/omap/mcpdm.h b/sound/soc/omap/mcpdm.h deleted file mode 100644 index b055ad1..0000000 --- a/sound/soc/omap/mcpdm.h +++ /dev/null @@ -1,152 +0,0 @@ -/* - * mcpdm.h -- Defines for McPDM driver - * - * Author: Jorge Eduardo Candelaria x0107209@ti.com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * version 2 as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA - * 02110-1301 USA - * - */ - -/* McPDM registers */ - -#define MCPDM_REVISION 0x00 -#define MCPDM_SYSCONFIG 0x10 -#define MCPDM_IRQSTATUS_RAW 0x24 -#define MCPDM_IRQSTATUS 0x28 -#define MCPDM_IRQENABLE_SET 0x2C -#define MCPDM_IRQENABLE_CLR 0x30 -#define MCPDM_IRQWAKE_EN 0x34 -#define MCPDM_DMAENABLE_SET 0x38 -#define MCPDM_DMAENABLE_CLR 0x3C -#define MCPDM_DMAWAKEEN 0x40 -#define MCPDM_CTRL 0x44 -#define MCPDM_DN_DATA 0x48 -#define MCPDM_UP_DATA 0x4C -#define MCPDM_FIFO_CTRL_DN 0x50 -#define MCPDM_FIFO_CTRL_UP 0x54 -#define MCPDM_DN_OFFSET 0x58 - -/* - * MCPDM_IRQ bit fields - * IRQSTATUS_RAW, IRQSTATUS, IRQENABLE_SET, IRQENABLE_CLR - */ - -#define MCPDM_DN_IRQ (1 << 0) -#define MCPDM_DN_IRQ_EMPTY (1 << 1) -#define MCPDM_DN_IRQ_ALMST_EMPTY (1 << 2) -#define MCPDM_DN_IRQ_FULL (1 << 3) - -#define MCPDM_UP_IRQ (1 << 8) -#define MCPDM_UP_IRQ_EMPTY (1 << 9) -#define MCPDM_UP_IRQ_ALMST_FULL (1 << 10) -#define MCPDM_UP_IRQ_FULL (1 << 11) - -#define MCPDM_DOWNLINK_IRQ_MASK 0x00F -#define MCPDM_UPLINK_IRQ_MASK 0xF00 - -/* - * MCPDM_DMAENABLE bit fields - */ - -#define DMA_DN_ENABLE 0x1 -#define DMA_UP_ENABLE 0x2 - -/* - * MCPDM_CTRL bit fields - */ - -#define PDM_UP1_EN 0x0001 -#define PDM_UP2_EN 0x0002 -#define PDM_UP3_EN 0x0004 -#define PDM_DN1_EN 0x0008 -#define PDM_DN2_EN 0x0010 -#define PDM_DN3_EN 0x0020 -#define PDM_DN4_EN 0x0040 -#define PDM_DN5_EN 0x0080 -#define PDMOUTFORMAT 0x0100 -#define CMD_INT 0x0200 -#define STATUS_INT 0x0400 -#define SW_UP_RST 0x0800 -#define SW_DN_RST 0x1000 -#define PDM_UP_MASK 0x007 -#define PDM_DN_MASK 0x0F8 -#define PDM_CMD_MASK 0x200 -#define PDM_STATUS_MASK 0x400 - - -#define PDMOUTFORMAT_LJUST (0 << 8) -#define PDMOUTFORMAT_RJUST (1 << 8) - -/* - * MCPDM_FIFO_CTRL bit fields - */ - -#define UP_THRES_MAX 0xF -#define DN_THRES_MAX 0xF - -/* - * MCPDM_DN_OFFSET bit fields - */ - -#define DN_OFST_RX1_EN 0x0001 -#define DN_OFST_RX2_EN 0x0100 - -#define DN_OFST_RX1 1 -#define DN_OFST_RX2 9 -#define DN_OFST_MAX 0x1F - -#define MCPDM_UPLINK 1 -#define MCPDM_DOWNLINK 2 - -struct omap_mcpdm_link { - int irq_mask; - int threshold; - int format; - int channels; -}; - -struct omap_mcpdm_platform_data { - unsigned long phys_base; - u16 irq; -}; - -struct omap_mcpdm { - struct device *dev; - unsigned long phys_base; - void __iomem *io_base; - u8 free; - int irq; - - spinlock_t lock; - struct omap_mcpdm_platform_data *pdata; - struct omap_mcpdm_link *downlink; - struct omap_mcpdm_link *uplink; - struct completion irq_completion; - - int dn_channels; - int up_channels; -}; - -extern void omap_mcpdm_start(int stream); -extern void omap_mcpdm_stop(int stream); -extern int omap_mcpdm_capture_open(struct omap_mcpdm_link *uplink); -extern int omap_mcpdm_playback_open(struct omap_mcpdm_link *downlink); -extern int omap_mcpdm_capture_close(struct omap_mcpdm_link *uplink); -extern int omap_mcpdm_playback_close(struct omap_mcpdm_link *downlink); -extern int omap_mcpdm_request(void); -extern void omap_mcpdm_free(void); -extern int omap_mcpdm_set_offset(int offset1, int offset2); -int __devinit omap_mcpdm_probe(struct platform_device *pdev); -int __devexit omap_mcpdm_remove(struct platform_device *pdev); diff --git a/sound/soc/omap/omap-mcpdm.c b/sound/soc/omap/omap-mcpdm.c dissimilarity index 46% index 0820b9e..4fd856d 100644 --- a/sound/soc/omap/omap-mcpdm.c +++ b/sound/soc/omap/omap-mcpdm.c @@ -1,279 +1,544 @@ -/* - * omap-mcpdm.c -- OMAP ALSA SoC DAI driver using McPDM port - * - * Copyright (C) 2009 Texas Instruments - * - * Author: Misael Lopez Cruz x0052729@ti.com - * Contact: Jorge Eduardo Candelaria x0107209@ti.com - * Margarita Olaya magi.olaya@ti.com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License - * version 2 as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA - * 02110-1301 USA - * - */ - -#include <linux/init.h> -#include <linux/module.h> -#include <linux/device.h> -#include <sound/core.h> -#include <sound/pcm.h> -#include <sound/pcm_params.h> -#include <sound/initval.h> -#include <sound/soc.h> - -#include <plat/dma.h> -#include <plat/mcbsp.h> -#include "mcpdm.h" -#include "omap-pcm.h" - -struct omap_mcpdm_data { - struct omap_mcpdm_link *links; - int active; -}; - -static struct omap_mcpdm_link omap_mcpdm_links[] = { - /* downlink */ - { - .irq_mask = MCPDM_DN_IRQ_EMPTY | MCPDM_DN_IRQ_FULL, - .threshold = 2, - .format = PDMOUTFORMAT_LJUST, - }, - /* uplink */ - { - .irq_mask = MCPDM_UP_IRQ_EMPTY | MCPDM_UP_IRQ_FULL, - .threshold = UP_THRES_MAX - 3, - .format = PDMOUTFORMAT_LJUST, - }, -}; - -static struct omap_mcpdm_data mcpdm_data = { - .links = omap_mcpdm_links, - .active = 0, -}; - -/* - * Stream DMA parameters - */ -static struct omap_pcm_dma_data omap_mcpdm_dai_dma_params[] = { - { - .name = "Audio playback", - .dma_req = OMAP44XX_DMA_MCPDM_DL, - .data_type = OMAP_DMA_DATA_TYPE_S32, - .sync_mode = OMAP_DMA_SYNC_PACKET, - .packet_size = 16, - .port_addr = OMAP44XX_MCPDM_L3_BASE + MCPDM_DN_DATA, - }, - { - .name = "Audio capture", - .dma_req = OMAP44XX_DMA_MCPDM_UP, - .data_type = OMAP_DMA_DATA_TYPE_S32, - .sync_mode = OMAP_DMA_SYNC_PACKET, - .packet_size = 16, - .port_addr = OMAP44XX_MCPDM_L3_BASE + MCPDM_UP_DATA, - }, -}; - -static int omap_mcpdm_dai_startup(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) -{ - int err = 0; - - if (!dai->active) - err = omap_mcpdm_request(); - - return err; -} - -static void omap_mcpdm_dai_shutdown(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) -{ - if (!dai->active) - omap_mcpdm_free(); -} - -static int omap_mcpdm_dai_trigger(struct snd_pcm_substream *substream, int cmd, - struct snd_soc_dai *dai) -{ - struct omap_mcpdm_data *mcpdm_priv = snd_soc_dai_get_drvdata(dai); - int stream = substream->stream; - int err = 0; - - switch (cmd) { - case SNDRV_PCM_TRIGGER_START: - case SNDRV_PCM_TRIGGER_RESUME: - case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - if (!mcpdm_priv->active++) - omap_mcpdm_start(stream); - break; - - case SNDRV_PCM_TRIGGER_STOP: - case SNDRV_PCM_TRIGGER_SUSPEND: - case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - if (!--mcpdm_priv->active) - omap_mcpdm_stop(stream); - break; - default: - err = -EINVAL; - } - - return err; -} - -static int omap_mcpdm_dai_hw_params(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params, - struct snd_soc_dai *dai) -{ - struct omap_mcpdm_data *mcpdm_priv = snd_soc_dai_get_drvdata(dai); - struct omap_mcpdm_link *mcpdm_links = mcpdm_priv->links; - struct omap_pcm_dma_data *dma_data; - int threshold; - int stream = substream->stream; - int channels, err, link_mask = 0; - - channels = params_channels(params); - switch (channels) { - case 4: - if (stream == SNDRV_PCM_STREAM_CAPTURE) - /* up to 2 channels for capture */ - return -EINVAL; - link_mask |= 1 << 3; - case 3: - if (stream == SNDRV_PCM_STREAM_CAPTURE) - /* up to 2 channels for capture */ - return -EINVAL; - link_mask |= 1 << 2; - case 2: - link_mask |= 1 << 1; - case 1: - link_mask |= 1 << 0; - break; - default: - /* unsupported number of channels */ - return -EINVAL; - } - - dma_data = &omap_mcpdm_dai_dma_params[stream]; - threshold = mcpdm_links[stream].threshold; - - if (stream == SNDRV_PCM_STREAM_PLAYBACK) { - mcpdm_links[stream].channels = link_mask << 3; - dma_data->packet_size = (DN_THRES_MAX - threshold) * channels; - - err = omap_mcpdm_playback_open(&mcpdm_links[stream]); - } else { - mcpdm_links[stream].channels = link_mask << 0; - dma_data->packet_size = threshold * channels; - - err = omap_mcpdm_capture_open(&mcpdm_links[stream]); - } - - snd_soc_dai_set_dma_data(dai, substream, dma_data); - return err; -} - -static int omap_mcpdm_dai_hw_free(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) -{ - struct omap_mcpdm_data *mcpdm_priv = snd_soc_dai_get_drvdata(dai); - struct omap_mcpdm_link *mcpdm_links = mcpdm_priv->links; - int stream = substream->stream; - int err; - - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - err = omap_mcpdm_playback_close(&mcpdm_links[stream]); - else - err = omap_mcpdm_capture_close(&mcpdm_links[stream]); - - return err; -} - -static struct snd_soc_dai_ops omap_mcpdm_dai_ops = { - .startup = omap_mcpdm_dai_startup, - .shutdown = omap_mcpdm_dai_shutdown, - .trigger = omap_mcpdm_dai_trigger, - .hw_params = omap_mcpdm_dai_hw_params, - .hw_free = omap_mcpdm_dai_hw_free, -}; - -#define OMAP_MCPDM_RATES (SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000) -#define OMAP_MCPDM_FORMATS (SNDRV_PCM_FMTBIT_S32_LE) - -static int omap_mcpdm_dai_probe(struct snd_soc_dai *dai) -{ - snd_soc_dai_set_drvdata(dai, &mcpdm_data); - return 0; -} - -static struct snd_soc_dai_driver omap_mcpdm_dai = { - .probe = omap_mcpdm_dai_probe, - .playback = { - .channels_min = 1, - .channels_max = 4, - .rates = OMAP_MCPDM_RATES, - .formats = OMAP_MCPDM_FORMATS, - }, - .capture = { - .channels_min = 1, - .channels_max = 2, - .rates = OMAP_MCPDM_RATES, - .formats = OMAP_MCPDM_FORMATS, - }, - .ops = &omap_mcpdm_dai_ops, -}; - -static __devinit int asoc_mcpdm_probe(struct platform_device *pdev) -{ - int ret; - - ret = omap_mcpdm_probe(pdev); - if (ret < 0) - return ret; - ret = snd_soc_register_dai(&pdev->dev, &omap_mcpdm_dai); - if (ret < 0) - omap_mcpdm_remove(pdev); - return ret; -} - -static int __devexit asoc_mcpdm_remove(struct platform_device *pdev) -{ - snd_soc_unregister_dai(&pdev->dev); - omap_mcpdm_remove(pdev); - return 0; -} - -static struct platform_driver asoc_mcpdm_driver = { - .driver = { - .name = "omap-mcpdm", - .owner = THIS_MODULE, - }, - - .probe = asoc_mcpdm_probe, - .remove = __devexit_p(asoc_mcpdm_remove), -}; - -static int __init snd_omap_mcpdm_init(void) -{ - return platform_driver_register(&asoc_mcpdm_driver); -} -module_init(snd_omap_mcpdm_init); - -static void __exit snd_omap_mcpdm_exit(void) -{ - platform_driver_unregister(&asoc_mcpdm_driver); -} -module_exit(snd_omap_mcpdm_exit); - -MODULE_AUTHOR("Misael Lopez Cruz x0052729@ti.com"); -MODULE_DESCRIPTION("OMAP PDM SoC Interface"); -MODULE_LICENSE("GPL"); +/* + * omap-mcpdm.c -- OMAP ALSA SoC DAI driver using McPDM port + * + * Copyright (C) 2009 - 2011 Texas Instruments + * + * Author: Misael Lopez Cruz misael.lopez@ti.com + * Contact: Jorge Eduardo Candelaria x0107209@ti.com + * Margarita Olaya magi.olaya@ti.com + * Peter Ujfalusi peter.ujfalusi@ti.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA + * + */ + +#include <linux/init.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/interrupt.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/irq.h> +#include <linux/slab.h> +#include <linux/pm_runtime.h> + +#include <sound/core.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> + +#include <plat/dma.h> +#include <plat/omap_hwmod.h> +#include "omap-mcpdm.h" +#include "omap-pcm.h" + +struct omap_mcpdm { + struct device *dev; + unsigned long phys_base; + void __iomem *io_base; + int irq; + + struct mutex mutex; + + /* channel data */ + u32 dn_channels; + u32 up_channels; + + /* McPDM FIFO thresholds */ + u32 dn_threshold; + u32 up_threshold; + + int enabled; +}; + +/* + * Stream DMA parameters + */ +static struct omap_pcm_dma_data omap_mcpdm_dai_dma_params[] = { + { + .name = "Audio playback", + .dma_req = OMAP44XX_DMA_MCPDM_DL, + .data_type = OMAP_DMA_DATA_TYPE_S32, + .sync_mode = OMAP_DMA_SYNC_PACKET, + .port_addr = OMAP44XX_MCPDM_L3_BASE + MCPDM_REG_DN_DATA, + }, + { + .name = "Audio capture", + .dma_req = OMAP44XX_DMA_MCPDM_UP, + .data_type = OMAP_DMA_DATA_TYPE_S32, + .sync_mode = OMAP_DMA_SYNC_PACKET, + .port_addr = OMAP44XX_MCPDM_L3_BASE + MCPDM_REG_UP_DATA, + }, +}; + +static inline void omap_mcpdm_write(struct omap_mcpdm *mcpdm, u16 reg, u32 val) +{ + __raw_writel(val, mcpdm->io_base + reg); +} + +static inline int omap_mcpdm_read(struct omap_mcpdm *mcpdm, u16 reg) +{ + return __raw_readl(mcpdm->io_base + reg); +} + +#ifdef DEBUG +static void omap_mcpdm_reg_dump(struct omap_mcpdm *mcpdm) +{ + dev_dbg(mcpdm->dev, "***********************\n"); + dev_dbg(mcpdm->dev, "IRQSTATUS_RAW: 0x%04x\n", + omap_mcpdm_read(mcpdm, MCPDM_REG_IRQSTATUS_RAW)); + dev_dbg(mcpdm->dev, "IRQSTATUS: 0x%04x\n", + omap_mcpdm_read(mcpdm, MCPDM_REG_IRQSTATUS)); + dev_dbg(mcpdm->dev, "IRQENABLE_SET: 0x%04x\n", + omap_mcpdm_read(mcpdm, MCPDM_REG_IRQENABLE_SET)); + dev_dbg(mcpdm->dev, "IRQENABLE_CLR: 0x%04x\n", + omap_mcpdm_read(mcpdm, MCPDM_REG_IRQENABLE_CLR)); + dev_dbg(mcpdm->dev, "IRQWAKE_EN: 0x%04x\n", + omap_mcpdm_read(mcpdm, MCPDM_REG_IRQWAKE_EN)); + dev_dbg(mcpdm->dev, "DMAENABLE_SET: 0x%04x\n", + omap_mcpdm_read(mcpdm, MCPDM_REG_DMAENABLE_SET)); + dev_dbg(mcpdm->dev, "DMAENABLE_CLR: 0x%04x\n", + omap_mcpdm_read(mcpdm, MCPDM_REG_DMAENABLE_CLR)); + dev_dbg(mcpdm->dev, "DMAWAKEEN: 0x%04x\n", + omap_mcpdm_read(mcpdm, MCPDM_REG_DMAWAKEEN)); + dev_dbg(mcpdm->dev, "CTRL: 0x%04x\n", + omap_mcpdm_read(mcpdm, MCPDM_REG_CTRL)); + dev_dbg(mcpdm->dev, "DN_DATA: 0x%04x\n", + omap_mcpdm_read(mcpdm, MCPDM_REG_DN_DATA)); + dev_dbg(mcpdm->dev, "UP_DATA: 0x%04x\n", + omap_mcpdm_read(mcpdm, MCPDM_REG_UP_DATA)); + dev_dbg(mcpdm->dev, "FIFO_CTRL_DN: 0x%04x\n", + omap_mcpdm_read(mcpdm, MCPDM_REG_FIFO_CTRL_DN)); + dev_dbg(mcpdm->dev, "FIFO_CTRL_UP: 0x%04x\n", + omap_mcpdm_read(mcpdm, MCPDM_REG_FIFO_CTRL_UP)); + dev_dbg(mcpdm->dev, "***********************\n"); +} +#else +static void omap_mcpdm_reg_dump(struct omap_mcpdm *mcpdm) {} +#endif + +/* + * 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); +} + +/* + * Disables the transfer through the PDM interface to/from the Phoenix + * codec by disabling the corresponding UP or DN channels. + */ +static void omap_mcpdm_stop(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); + +} + +/* + * Is the physical McPDM interface active. + */ +static inline int omap_mcpdm_active(struct omap_mcpdm *mcpdm) +{ + return omap_mcpdm_read(mcpdm, MCPDM_REG_CTRL) & + (MCPDM_PDM_DN_MASK | MCPDM_PDM_UP_MASK); +} + +/* + * Configures McPDM uplink, and downlink for audio. + * This function should be called before omap_mcpdm_start. + */ +static void omap_mcpdm_open_streams(struct omap_mcpdm *mcpdm) +{ + omap_mcpdm_write(mcpdm, MCPDM_REG_IRQENABLE_SET, + MCPDM_DN_IRQ_EMPTY | MCPDM_DN_IRQ_FULL | + MCPDM_UP_IRQ_EMPTY | MCPDM_UP_IRQ_FULL); + + omap_mcpdm_write(mcpdm, MCPDM_REG_FIFO_CTRL_DN, mcpdm->dn_threshold); + omap_mcpdm_write(mcpdm, MCPDM_REG_FIFO_CTRL_UP, mcpdm->up_threshold); + + omap_mcpdm_write(mcpdm, MCPDM_REG_DMAENABLE_SET, + MCPDM_DMA_DN_ENABLE | MCPDM_DMA_UP_ENABLE); +} + +/* + * Cleans McPDM uplink, and downlink configuration. + * This function should be called when the stream is closed. + */ +static void omap_mcpdm_close_streams(struct omap_mcpdm *mcpdm) +{ + /* Disable irq request generation for downlink */ + omap_mcpdm_write(mcpdm, MCPDM_REG_IRQENABLE_CLR, + MCPDM_DN_IRQ_EMPTY | MCPDM_DN_IRQ_FULL); + + /* Disable DMA request generation for downlink */ + omap_mcpdm_write(mcpdm, MCPDM_REG_DMAENABLE_CLR, MCPDM_DMA_DN_ENABLE); + + /* Disable irq request generation for uplink */ + omap_mcpdm_write(mcpdm, MCPDM_REG_IRQENABLE_CLR, + MCPDM_UP_IRQ_EMPTY | MCPDM_UP_IRQ_FULL); + + /* Disable DMA request generation for uplink */ + omap_mcpdm_write(mcpdm, MCPDM_REG_DMAENABLE_CLR, MCPDM_DMA_UP_ENABLE); +} + +static irqreturn_t omap_mcpdm_irq_handler(int irq, void *dev_id) +{ + struct omap_mcpdm *mcpdm = dev_id; + int irq_status; + + irq_status = omap_mcpdm_read(mcpdm, MCPDM_REG_IRQSTATUS); + + /* Acknowledge irq event */ + omap_mcpdm_write(mcpdm, MCPDM_REG_IRQSTATUS, irq_status); + + if (irq_status & MCPDM_DN_IRQ_FULL) + dev_dbg(mcpdm->dev, "DN (playback) FIFO Full\n"); + + if (irq_status & MCPDM_DN_IRQ_EMPTY) + dev_dbg(mcpdm->dev, "DN (playback) FIFO Empty\n"); + + if (irq_status & MCPDM_DN_IRQ) + dev_dbg(mcpdm->dev, "DN (playback) write request\n"); + + if (irq_status & MCPDM_UP_IRQ_FULL) + dev_dbg(mcpdm->dev, "UP (capture) FIFO Full\n"); + + if (irq_status & MCPDM_UP_IRQ_EMPTY) + dev_dbg(mcpdm->dev, "UP (capture) FIFO Empty\n"); + + if (irq_status & MCPDM_UP_IRQ) + dev_dbg(mcpdm->dev, "UP (capture) write request\n"); + + return IRQ_HANDLED; +} + +static int omap_mcpdm_dai_startup(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct omap_mcpdm *mcpdm = snd_soc_dai_get_drvdata(dai); + + mutex_lock(&mcpdm->mutex); + + if (!mcpdm->enabled) { + pm_runtime_get_sync(mcpdm->dev); + + /* Enable watch dog for ES above ES 1.0 to avoid saturation */ + if (omap_rev() != OMAP4430_REV_ES1_0) { + u32 ctrl = omap_mcpdm_read(mcpdm, MCPDM_REG_CTRL); + + omap_mcpdm_write(mcpdm, MCPDM_REG_CTRL, + ctrl | MCPDM_WD_EN); + } + omap_mcpdm_open_streams(mcpdm); + mcpdm->enabled = 1; + } + + mutex_unlock(&mcpdm->mutex); + + return 0; +} + +static int omap_mcpdm_dai_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + struct omap_mcpdm *mcpdm = snd_soc_dai_get_drvdata(dai); + int stream = substream->stream; + struct omap_pcm_dma_data *dma_data; + int channels; + int link_mask = 0; + + channels = params_channels(params); + switch (channels) { + case 4: + if (stream == SNDRV_PCM_STREAM_CAPTURE) + /* up to 2 channels for capture */ + return -EINVAL; + link_mask |= 1 << 3; + case 3: + if (stream == SNDRV_PCM_STREAM_CAPTURE) + /* up to 2 channels for capture */ + return -EINVAL; + link_mask |= 1 << 2; + case 2: + link_mask |= 1 << 1; + case 1: + link_mask |= 1 << 0; + break; + default: + /* unsupported number of channels */ + return -EINVAL; + } + + dma_data = &omap_mcpdm_dai_dma_params[stream]; + + /* Configure McPDM channels, and DMA packet size */ + if (stream == SNDRV_PCM_STREAM_PLAYBACK) { + mcpdm->dn_channels = link_mask << 3; + dma_data->packet_size = + (MCPDM_DN_THRES_MAX - mcpdm->dn_threshold) * channels; + } else { + mcpdm->up_channels = link_mask << 0; + dma_data->packet_size = mcpdm->up_threshold * channels; + } + + snd_soc_dai_set_dma_data(dai, substream, dma_data); + + return 0; +} + +static int omap_mcpdm_prepare(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct omap_mcpdm *mcpdm = snd_soc_dai_get_drvdata(dai); + + if (!omap_mcpdm_active(mcpdm)) { + omap_mcpdm_start(mcpdm); + omap_mcpdm_reg_dump(mcpdm); + } + + return 0; +} + +static struct snd_soc_dai_ops omap_mcpdm_dai_ops = { + .startup = omap_mcpdm_dai_startup, + .hw_params = omap_mcpdm_dai_hw_params, + .prepare = omap_mcpdm_prepare, +}; + +static int omap_mcpdm_probe(struct snd_soc_dai *dai) +{ + struct omap_mcpdm *mcpdm = snd_soc_dai_get_drvdata(dai); + int ret; + + pm_runtime_enable(mcpdm->dev); + + /* Disable lines while request is ongoing */ + pm_runtime_get_sync(mcpdm->dev); + omap_mcpdm_write(mcpdm, MCPDM_REG_CTRL, 0x00); + + ret = request_irq(mcpdm->irq, omap_mcpdm_irq_handler, + 0, "McPDM", (void *)mcpdm); + + pm_runtime_put_sync(mcpdm->dev); + + if (ret) { + dev_err(mcpdm->dev, "Request for IRQ failed\n"); + pm_runtime_disable(mcpdm->dev); + } + + /* Configure McPDM threshold values */ + mcpdm->dn_threshold = 2; + mcpdm->up_threshold = MCPDM_UP_THRES_MAX - 3; + return ret; +} + +static int omap_mcpdm_remove(struct snd_soc_dai *dai) +{ + struct omap_mcpdm *mcpdm = snd_soc_dai_get_drvdata(dai); + + free_irq(mcpdm->irq, (void *)mcpdm); + pm_runtime_disable(mcpdm->dev); + + return 0; +} + +#define OMAP_MCPDM_RATES (SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000) +#define OMAP_MCPDM_FORMATS SNDRV_PCM_FMTBIT_S32_LE + +static struct snd_soc_dai_driver omap_mcpdm_dai = { + .probe = omap_mcpdm_probe, + .remove = omap_mcpdm_remove, + .probe_order = SND_SOC_COMP_ORDER_LATE, + .remove_order = SND_SOC_COMP_ORDER_EARLY, + .playback = { + .channels_min = 1, + .channels_max = 4, + .rates = OMAP_MCPDM_RATES, + .formats = OMAP_MCPDM_FORMATS, + }, + .capture = { + .channels_min = 1, + .channels_max = 2, + .rates = OMAP_MCPDM_RATES, + .formats = OMAP_MCPDM_FORMATS, + }, + .ops = &omap_mcpdm_dai_ops, +}; + +/* + * DAPM event function to ensure, that the host side McPDM interface is turned + * off after the codec's DAC. + */ +static int omap_mcpdm_interface_event(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + struct omap_mcpdm *mcpdm = w->private_data; + + mutex_lock(&mcpdm->mutex); + switch (event) { + case SND_SOC_DAPM_POST_PMD: + if (unlikely(!mcpdm->enabled)) { + dev_err(mcpdm->dev, "%s: Not enabled\n", __func__); + goto out; + } + if (omap_mcpdm_active(mcpdm)) { + omap_mcpdm_stop(mcpdm); + omap_mcpdm_close_streams(mcpdm); + } + + if (!omap_mcpdm_active(mcpdm)) + pm_runtime_put_sync(mcpdm->dev); + + mcpdm->enabled = 0; + break; + } +out: + mutex_unlock(&mcpdm->mutex); + return 0; +} + +/* + * DAPM_SUPPLY widget can be hooked up in the machine driver to make sure that + * the host side McPDM is not turned off too early. + */ +static struct snd_soc_dapm_widget omap_mcpdm_widgets[] = { + SND_SOC_DAPM_SUPPLY("OMAP McPDM Interface", SND_SOC_NOPM, 0, 0, + omap_mcpdm_interface_event, SND_SOC_DAPM_POST_PMD), +}; + +int omap_mcpdm_add_dapm_widget(struct snd_soc_codec *codec) +{ + struct snd_soc_dapm_context *dapm = &codec->dapm; + + return snd_soc_dapm_new_controls(dapm, omap_mcpdm_widgets, + ARRAY_SIZE(omap_mcpdm_widgets)); +} + +static __devinit int asoc_mcpdm_probe(struct platform_device *pdev) +{ + struct omap_mcpdm *mcpdm; + struct resource *res; + int ret = 0; + + mcpdm = kzalloc(sizeof(struct omap_mcpdm), GFP_KERNEL); + if (!mcpdm) + return -ENOMEM; + + platform_set_drvdata(pdev, mcpdm); + + mutex_init(&mcpdm->mutex); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (res == NULL) { + dev_err(&pdev->dev, "no resource\n"); + goto err_res; + } + + if (!request_mem_region(res->start, resource_size(res), "McPDM")) { + ret = -EBUSY; + goto err_res; + } + + mcpdm->io_base = ioremap(res->start, resource_size(res)); + if (!mcpdm->io_base) { + ret = -ENOMEM; + goto err_iomap; + } + + mcpdm->irq = platform_get_irq(pdev, 0); + if (mcpdm->irq < 0) { + ret = mcpdm->irq; + goto err_irq; + } + + mcpdm->dev = &pdev->dev; + + ret = snd_soc_register_dai(&pdev->dev, &omap_mcpdm_dai); + if (!ret) { + /* Initialize the private_data within the DAPM_SUPPLY widget */ + omap_mcpdm_widgets[0].private_data = mcpdm; + return 0; + } + +err_irq: + iounmap(mcpdm->io_base); +err_iomap: + release_mem_region(res->start, resource_size(res)); +err_res: + kfree(mcpdm); + return ret; +} + +static int __devexit asoc_mcpdm_remove(struct platform_device *pdev) +{ + struct omap_mcpdm *mcpdm = platform_get_drvdata(pdev); + struct resource *res; + + snd_soc_unregister_dai(&pdev->dev); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + iounmap(mcpdm->io_base); + release_mem_region(res->start, resource_size(res)); + + kfree(mcpdm); + return 0; +} + +static struct platform_driver asoc_mcpdm_driver = { + .driver = { + .name = "omap-mcpdm", + .owner = THIS_MODULE, + }, + + .probe = asoc_mcpdm_probe, + .remove = __devexit_p(asoc_mcpdm_remove), +}; + +static int __init snd_omap_mcpdm_init(void) +{ + return platform_driver_register(&asoc_mcpdm_driver); +} +module_init(snd_omap_mcpdm_init); + +static void __exit snd_omap_mcpdm_exit(void) +{ + platform_driver_unregister(&asoc_mcpdm_driver); +} +module_exit(snd_omap_mcpdm_exit); + +MODULE_AUTHOR("Misael Lopez Cruz misael.lopez@ti.com"); +MODULE_DESCRIPTION("OMAP PDM SoC Interface"); +MODULE_LICENSE("GPL"); diff --git a/sound/soc/omap/omap-mcpdm.h b/sound/soc/omap/omap-mcpdm.h new file mode 100644 index 0000000..e72524c --- /dev/null +++ b/sound/soc/omap/omap-mcpdm.h @@ -0,0 +1,97 @@ +/* + * omap-mcpdm.h + * + * Copyright (C) 2009 - 2011 Texas Instruments + * + * Contact: Misael Lopez Cruz misael.lopez@ti.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA + * + */ + +#ifndef __OMAP_MCPDM_H__ +#define __OMAP_MCPDM_H__ + +#define MCPDM_REG_REVISION 0x00 +#define MCPDM_REG_SYSCONFIG 0x10 +#define MCPDM_REG_IRQSTATUS_RAW 0x24 +#define MCPDM_REG_IRQSTATUS 0x28 +#define MCPDM_REG_IRQENABLE_SET 0x2C +#define MCPDM_REG_IRQENABLE_CLR 0x30 +#define MCPDM_REG_IRQWAKE_EN 0x34 +#define MCPDM_REG_DMAENABLE_SET 0x38 +#define MCPDM_REG_DMAENABLE_CLR 0x3C +#define MCPDM_REG_DMAWAKEEN 0x40 +#define MCPDM_REG_CTRL 0x44 +#define MCPDM_REG_DN_DATA 0x48 +#define MCPDM_REG_UP_DATA 0x4C +#define MCPDM_REG_FIFO_CTRL_DN 0x50 +#define MCPDM_REG_FIFO_CTRL_UP 0x54 +#define MCPDM_REG_DN_OFFSET 0x58 + +/* + * MCPDM_IRQ bit fields + * IRQSTATUS_RAW, IRQSTATUS, IRQENABLE_SET, IRQENABLE_CLR + */ + +#define MCPDM_DN_IRQ (1 << 0) +#define MCPDM_DN_IRQ_EMPTY (1 << 1) +#define MCPDM_DN_IRQ_ALMST_EMPTY (1 << 2) +#define MCPDM_DN_IRQ_FULL (1 << 3) + +#define MCPDM_UP_IRQ (1 << 8) +#define MCPDM_UP_IRQ_EMPTY (1 << 9) +#define MCPDM_UP_IRQ_ALMST_FULL (1 << 10) +#define MCPDM_UP_IRQ_FULL (1 << 11) + +#define MCPDM_DOWNLINK_IRQ_MASK 0x00F +#define MCPDM_UPLINK_IRQ_MASK 0xF00 + +/* + * MCPDM_DMAENABLE bit fields + */ + +#define MCPDM_DMA_DN_ENABLE (1 << 0) +#define MCPDM_DMA_UP_ENABLE (1 << 1) + +/* + * MCPDM_CTRL bit fields + */ + +#define MCPDM_PDM_UPLINK_EN(x) (1 << (x - 1)) /* ch1 is at bit 0 */ +#define MCPDM_PDM_DOWNLINK_EN(x) (1 << (x + 2)) /* ch1 is at bit 3 */ +#define MCPDM_PDMOUTFORMAT (1 << 8) +#define MCPDM_CMD_INT (1 << 9) +#define MCPDM_STATUS_INT (1 << 10) +#define MCPDM_SW_UP_RST (1 << 11) +#define MCPDM_SW_DN_RST (1 << 12) +#define MCPDM_WD_EN (1 << 14) +#define MCPDM_PDM_UP_MASK 0x7 +#define MCPDM_PDM_DN_MASK (0x1f << 3) + + +#define MCPDM_PDMOUTFORMAT_LJUST (0 << 8) +#define MCPDM_PDMOUTFORMAT_RJUST (1 << 8) + +/* + * MCPDM_FIFO_CTRL bit fields + */ + +#define MCPDM_UP_THRES_MAX 0xF +#define MCPDM_DN_THRES_MAX 0xF + +int omap_mcpdm_add_dapm_widget(struct snd_soc_codec *codec); + +#endif /* End of __OMAP_MCPDM_H__ */ diff --git a/sound/soc/omap/sdp4430.c b/sound/soc/omap/sdp4430.c index eec77fe..a2b4f60 100644 --- a/sound/soc/omap/sdp4430.c +++ b/sound/soc/omap/sdp4430.c @@ -32,7 +32,7 @@ #include <plat/hardware.h> #include <plat/mux.h>
-#include "mcpdm.h" +#include "omap-mcpdm.h" #include "omap-pcm.h" #include "../codecs/twl6040.h"
@@ -115,6 +115,14 @@ static const struct snd_soc_dapm_route audio_map[] = { /* Aux/FM Stereo In: AFML, AFMR */ {"AFML", NULL, "FM Stereo In"}, {"AFMR", NULL, "FM Stereo In"}, + + /* OMAP4 McPDM interface */ + {"ADC Left", NULL, "OMAP McPDM Interface"}, + {"ADC Right", NULL, "OMAP McPDM Interface"}, + {"HSDAC Left", NULL, "OMAP McPDM Interface"}, + {"HSDAC Right", NULL, "OMAP McPDM Interface"}, + {"HFDAC Left", NULL, "OMAP McPDM Interface"}, + {"HFDAC Right", NULL, "OMAP McPDM Interface"}, };
static int sdp4430_twl6040_init(struct snd_soc_pcm_runtime *rtd) @@ -129,6 +137,10 @@ static int sdp4430_twl6040_init(struct snd_soc_pcm_runtime *rtd) if (ret) return ret;
+ ret = omap_mcpdm_add_dapm_widget(codec); + if (ret) + return ret; + /* Set up SDP4430 specific audio path audio_map */ snd_soc_dapm_add_routes(dapm, audio_map, ARRAY_SIZE(audio_map));
On 08/19/2011 09:41 AM, Peter Ujfalusi wrote:
From: Misael Lopez Cruz misael.lopez@ti.com
Reasons for the replacement: The current driver for McPDM was developed to support the legacy mode only. In preparation for the ABE support the current driver stack need the be replaced. The new driver is much simpler, easier to extend, and it also fixes some of the issues with the old stack.
Main changes:
- single file for omap-mcpdm (mcpdm.c/h removed)
- Define names for registers, bits cleaned up, prefixed
- Full-duplex audio operation (arecord | aplay) has been fixed
- Less code
McPDM need to be turned off _after_ the codec's DAC/ADC has been powered down. The solution for this without extensive change in core: Use DAPM_SUPPLY to turn off the McPDM interface. The machine driver connects the "OMAP McPDM Interface" to the codec's DAC/ADC, so after audio activity we can be sure that the host side McPDM is turned off at the correct time.
Signed-off-by: Misael Lopez Cruz misael.lopez@ti.com Signed-off-by: Liam Girdwood lrg@ti.com Signed-off-by: Sebastien Guiriec s-guiriec@ti.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
sound/soc/omap/Makefile | 2 +- sound/soc/omap/mcpdm.c | 472 ------------------------- sound/soc/omap/mcpdm.h | 152 -------- sound/soc/omap/omap-mcpdm.c | 823 ++++++++++++++++++++++++++++--------------- sound/soc/omap/omap-mcpdm.h | 97 +++++ sound/soc/omap/sdp4430.c | 14 +- 6 files changed, 655 insertions(+), 905 deletions(-) delete mode 100644 sound/soc/omap/mcpdm.c delete mode 100644 sound/soc/omap/mcpdm.h rewrite sound/soc/omap/omap-mcpdm.c (46%) create mode 100644 sound/soc/omap/omap-mcpdm.h
diff --git a/sound/soc/omap/Makefile b/sound/soc/omap/Makefile index 59e2c8d..052fd75 100644 --- a/sound/soc/omap/Makefile +++ b/sound/soc/omap/Makefile @@ -1,7 +1,7 @@ # OMAP Platform Support snd-soc-omap-objs := omap-pcm.o snd-soc-omap-mcbsp-objs := omap-mcbsp.o -snd-soc-omap-mcpdm-objs := omap-mcpdm.o mcpdm.o +snd-soc-omap-mcpdm-objs := omap-mcpdm.o snd-soc-omap-hdmi-objs := omap-hdmi.o
obj-$(CONFIG_SND_OMAP_SOC) += snd-soc-omap.o diff --git a/sound/soc/omap/mcpdm.c b/sound/soc/omap/mcpdm.c deleted file mode 100644 index 9746a49..0000000 --- a/sound/soc/omap/mcpdm.c +++ /dev/null @@ -1,472 +0,0 @@ [...] +static int omap_mcpdm_interface_event(struct snd_soc_dapm_widget *w,
struct snd_kcontrol *kcontrol, int event)
+{
- struct omap_mcpdm *mcpdm = w->private_data;
Can't you use struct omap_mcpdm *mcpdm = snd_soc_codec_get_drvdata(codec)?
This would avoid having to add the private_data field to the widget struct. In it's current form it will only really work, if there is just one instance of the driver using the widget. And if that's the case you can use a global variable directly anyway.
- Lars
On 08/19/2011 10:38 AM, Lars-Peter Clausen wrote:
On 08/19/2011 09:41 AM, Peter Ujfalusi wrote:
From: Misael Lopez Cruz misael.lopez@ti.com
Reasons for the replacement: The current driver for McPDM was developed to support the legacy mode only. In preparation for the ABE support the current driver stack need the be replaced. The new driver is much simpler, easier to extend, and it also fixes some of the issues with the old stack.
Main changes:
- single file for omap-mcpdm (mcpdm.c/h removed)
- Define names for registers, bits cleaned up, prefixed
- Full-duplex audio operation (arecord | aplay) has been fixed
- Less code
McPDM need to be turned off _after_ the codec's DAC/ADC has been powered down. The solution for this without extensive change in core: Use DAPM_SUPPLY to turn off the McPDM interface. The machine driver connects the "OMAP McPDM Interface" to the codec's DAC/ADC, so after audio activity we can be sure that the host side McPDM is turned off at the correct time.
Signed-off-by: Misael Lopez Cruz misael.lopez@ti.com Signed-off-by: Liam Girdwood lrg@ti.com Signed-off-by: Sebastien Guiriec s-guiriec@ti.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
sound/soc/omap/Makefile | 2 +- sound/soc/omap/mcpdm.c | 472 ------------------------- sound/soc/omap/mcpdm.h | 152 -------- sound/soc/omap/omap-mcpdm.c | 823 ++++++++++++++++++++++++++++--------------- sound/soc/omap/omap-mcpdm.h | 97 +++++ sound/soc/omap/sdp4430.c | 14 +- 6 files changed, 655 insertions(+), 905 deletions(-) delete mode 100644 sound/soc/omap/mcpdm.c delete mode 100644 sound/soc/omap/mcpdm.h rewrite sound/soc/omap/omap-mcpdm.c (46%) create mode 100644 sound/soc/omap/omap-mcpdm.h
diff --git a/sound/soc/omap/Makefile b/sound/soc/omap/Makefile index 59e2c8d..052fd75 100644 --- a/sound/soc/omap/Makefile +++ b/sound/soc/omap/Makefile @@ -1,7 +1,7 @@ # OMAP Platform Support snd-soc-omap-objs := omap-pcm.o snd-soc-omap-mcbsp-objs := omap-mcbsp.o -snd-soc-omap-mcpdm-objs := omap-mcpdm.o mcpdm.o +snd-soc-omap-mcpdm-objs := omap-mcpdm.o snd-soc-omap-hdmi-objs := omap-hdmi.o
obj-$(CONFIG_SND_OMAP_SOC) += snd-soc-omap.o diff --git a/sound/soc/omap/mcpdm.c b/sound/soc/omap/mcpdm.c deleted file mode 100644 index 9746a49..0000000 --- a/sound/soc/omap/mcpdm.c +++ /dev/null @@ -1,472 +0,0 @@ [...] +static int omap_mcpdm_interface_event(struct snd_soc_dapm_widget *w,
struct snd_kcontrol *kcontrol, int event)
+{
- struct omap_mcpdm *mcpdm = w->private_data;
Can't you use struct omap_mcpdm *mcpdm = snd_soc_codec_get_drvdata(codec)?
sorry: struct omap_mcpdm *mcpdm = snd_soc_platform_get_drvdata(w->platform);
This would avoid having to add the private_data field to the widget struct. In it's current form it will only really work, if there is just one instance of the driver using the widget. And if that's the case you can use a global variable directly anyway.
- Lars
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On 19/08/11 09:49, Lars-Peter Clausen wrote:
On 08/19/2011 10:38 AM, Lars-Peter Clausen wrote:
On 08/19/2011 09:41 AM, Peter Ujfalusi wrote:
From: Misael Lopez Cruz misael.lopez@ti.com
Reasons for the replacement: The current driver for McPDM was developed to support the legacy mode only. In preparation for the ABE support the current driver stack need the be replaced. The new driver is much simpler, easier to extend, and it also fixes some of the issues with the old stack.
Main changes:
- single file for omap-mcpdm (mcpdm.c/h removed)
- Define names for registers, bits cleaned up, prefixed
- Full-duplex audio operation (arecord | aplay) has been fixed
- Less code
McPDM need to be turned off _after_ the codec's DAC/ADC has been powered down. The solution for this without extensive change in core: Use DAPM_SUPPLY to turn off the McPDM interface. The machine driver connects the "OMAP McPDM Interface" to the codec's DAC/ADC, so after audio activity we can be sure that the host side McPDM is turned off at the correct time.
Signed-off-by: Misael Lopez Cruz misael.lopez@ti.com Signed-off-by: Liam Girdwood lrg@ti.com Signed-off-by: Sebastien Guiriec s-guiriec@ti.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
sound/soc/omap/Makefile | 2 +- sound/soc/omap/mcpdm.c | 472 ------------------------- sound/soc/omap/mcpdm.h | 152 -------- sound/soc/omap/omap-mcpdm.c | 823 ++++++++++++++++++++++++++++--------------- sound/soc/omap/omap-mcpdm.h | 97 +++++ sound/soc/omap/sdp4430.c | 14 +- 6 files changed, 655 insertions(+), 905 deletions(-) delete mode 100644 sound/soc/omap/mcpdm.c delete mode 100644 sound/soc/omap/mcpdm.h rewrite sound/soc/omap/omap-mcpdm.c (46%) create mode 100644 sound/soc/omap/omap-mcpdm.h
diff --git a/sound/soc/omap/Makefile b/sound/soc/omap/Makefile index 59e2c8d..052fd75 100644 --- a/sound/soc/omap/Makefile +++ b/sound/soc/omap/Makefile @@ -1,7 +1,7 @@ # OMAP Platform Support snd-soc-omap-objs := omap-pcm.o snd-soc-omap-mcbsp-objs := omap-mcbsp.o -snd-soc-omap-mcpdm-objs := omap-mcpdm.o mcpdm.o +snd-soc-omap-mcpdm-objs := omap-mcpdm.o snd-soc-omap-hdmi-objs := omap-hdmi.o
obj-$(CONFIG_SND_OMAP_SOC) += snd-soc-omap.o diff --git a/sound/soc/omap/mcpdm.c b/sound/soc/omap/mcpdm.c deleted file mode 100644 index 9746a49..0000000 --- a/sound/soc/omap/mcpdm.c +++ /dev/null @@ -1,472 +0,0 @@ [...] +static int omap_mcpdm_interface_event(struct snd_soc_dapm_widget *w,
struct snd_kcontrol *kcontrol, int event)
+{
- struct omap_mcpdm *mcpdm = w->private_data;
Can't you use struct omap_mcpdm *mcpdm = snd_soc_codec_get_drvdata(codec)?
sorry: struct omap_mcpdm *mcpdm = snd_soc_platform_get_drvdata(w->platform);
This would avoid having to add the private_data field to the widget struct. In it's current form it will only really work, if there is just one instance of the driver using the widget. And if that's the case you can use a global variable directly anyway.
This wont work as the McPDM DAI is neither a codec or a platform driver. It's best to give the widgets some private data since we may have other uses for it too.
Thanks
Liam
On 08/19/2011 02:38 PM, Liam Girdwood wrote:
On 19/08/11 09:49, Lars-Peter Clausen wrote:
On 08/19/2011 10:38 AM, Lars-Peter Clausen wrote:
On 08/19/2011 09:41 AM, Peter Ujfalusi wrote:
From: Misael Lopez Cruz misael.lopez@ti.com
Reasons for the replacement: The current driver for McPDM was developed to support the legacy mode only. In preparation for the ABE support the current driver stack need the be replaced. The new driver is much simpler, easier to extend, and it also fixes some of the issues with the old stack.
Main changes:
- single file for omap-mcpdm (mcpdm.c/h removed)
- Define names for registers, bits cleaned up, prefixed
- Full-duplex audio operation (arecord | aplay) has been fixed
- Less code
McPDM need to be turned off _after_ the codec's DAC/ADC has been powered down. The solution for this without extensive change in core: Use DAPM_SUPPLY to turn off the McPDM interface. The machine driver connects the "OMAP McPDM Interface" to the codec's DAC/ADC, so after audio activity we can be sure that the host side McPDM is turned off at the correct time.
Signed-off-by: Misael Lopez Cruz misael.lopez@ti.com Signed-off-by: Liam Girdwood lrg@ti.com Signed-off-by: Sebastien Guiriec s-guiriec@ti.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
sound/soc/omap/Makefile | 2 +- sound/soc/omap/mcpdm.c | 472 ------------------------- sound/soc/omap/mcpdm.h | 152 -------- sound/soc/omap/omap-mcpdm.c | 823 ++++++++++++++++++++++++++++--------------- sound/soc/omap/omap-mcpdm.h | 97 +++++ sound/soc/omap/sdp4430.c | 14 +- 6 files changed, 655 insertions(+), 905 deletions(-) delete mode 100644 sound/soc/omap/mcpdm.c delete mode 100644 sound/soc/omap/mcpdm.h rewrite sound/soc/omap/omap-mcpdm.c (46%) create mode 100644 sound/soc/omap/omap-mcpdm.h
diff --git a/sound/soc/omap/Makefile b/sound/soc/omap/Makefile index 59e2c8d..052fd75 100644 --- a/sound/soc/omap/Makefile +++ b/sound/soc/omap/Makefile @@ -1,7 +1,7 @@ # OMAP Platform Support snd-soc-omap-objs := omap-pcm.o snd-soc-omap-mcbsp-objs := omap-mcbsp.o -snd-soc-omap-mcpdm-objs := omap-mcpdm.o mcpdm.o +snd-soc-omap-mcpdm-objs := omap-mcpdm.o snd-soc-omap-hdmi-objs := omap-hdmi.o
obj-$(CONFIG_SND_OMAP_SOC) += snd-soc-omap.o diff --git a/sound/soc/omap/mcpdm.c b/sound/soc/omap/mcpdm.c deleted file mode 100644 index 9746a49..0000000 --- a/sound/soc/omap/mcpdm.c +++ /dev/null @@ -1,472 +0,0 @@ [...] +static int omap_mcpdm_interface_event(struct snd_soc_dapm_widget *w,
struct snd_kcontrol *kcontrol, int event)
+{
- struct omap_mcpdm *mcpdm = w->private_data;
Can't you use struct omap_mcpdm *mcpdm = snd_soc_codec_get_drvdata(codec)?
sorry: struct omap_mcpdm *mcpdm = snd_soc_platform_get_drvdata(w->platform);
This would avoid having to add the private_data field to the widget struct. In it's current form it will only really work, if there is just one instance of the driver using the widget. And if that's the case you can use a global variable directly anyway.
This wont work as the McPDM DAI is neither a codec or a platform driver. It's best to give the widgets some private data since we may have other uses for it too.
oops, sorry. I somehow though this was a platform driver, don't know why. But still the way you use private_data in this driver, you use it to cleverly hide a global variable.
The whole thing is sort of a hack. I mean, if it turns out that we want DAPM on the SoC DAI side, we should probably give it is own DAPM context, instead of using the cards context for this.
- Lars
On 19/08/11 14:13, Lars-Peter Clausen wrote:
On 08/19/2011 02:38 PM, Liam Girdwood wrote:
On 19/08/11 09:49, Lars-Peter Clausen wrote:
On 08/19/2011 10:38 AM, Lars-Peter Clausen wrote:
On 08/19/2011 09:41 AM, Peter Ujfalusi wrote:
From: Misael Lopez Cruz misael.lopez@ti.com
Reasons for the replacement: The current driver for McPDM was developed to support the legacy mode only. In preparation for the ABE support the current driver stack need the be replaced. The new driver is much simpler, easier to extend, and it also fixes some of the issues with the old stack.
Main changes:
- single file for omap-mcpdm (mcpdm.c/h removed)
- Define names for registers, bits cleaned up, prefixed
- Full-duplex audio operation (arecord | aplay) has been fixed
- Less code
McPDM need to be turned off _after_ the codec's DAC/ADC has been powered down. The solution for this without extensive change in core: Use DAPM_SUPPLY to turn off the McPDM interface. The machine driver connects the "OMAP McPDM Interface" to the codec's DAC/ADC, so after audio activity we can be sure that the host side McPDM is turned off at the correct time.
Signed-off-by: Misael Lopez Cruz misael.lopez@ti.com Signed-off-by: Liam Girdwood lrg@ti.com Signed-off-by: Sebastien Guiriec s-guiriec@ti.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
sound/soc/omap/Makefile | 2 +- sound/soc/omap/mcpdm.c | 472 ------------------------- sound/soc/omap/mcpdm.h | 152 -------- sound/soc/omap/omap-mcpdm.c | 823 ++++++++++++++++++++++++++++--------------- sound/soc/omap/omap-mcpdm.h | 97 +++++ sound/soc/omap/sdp4430.c | 14 +- 6 files changed, 655 insertions(+), 905 deletions(-) delete mode 100644 sound/soc/omap/mcpdm.c delete mode 100644 sound/soc/omap/mcpdm.h rewrite sound/soc/omap/omap-mcpdm.c (46%) create mode 100644 sound/soc/omap/omap-mcpdm.h
diff --git a/sound/soc/omap/Makefile b/sound/soc/omap/Makefile index 59e2c8d..052fd75 100644 --- a/sound/soc/omap/Makefile +++ b/sound/soc/omap/Makefile @@ -1,7 +1,7 @@ # OMAP Platform Support snd-soc-omap-objs := omap-pcm.o snd-soc-omap-mcbsp-objs := omap-mcbsp.o -snd-soc-omap-mcpdm-objs := omap-mcpdm.o mcpdm.o +snd-soc-omap-mcpdm-objs := omap-mcpdm.o snd-soc-omap-hdmi-objs := omap-hdmi.o
obj-$(CONFIG_SND_OMAP_SOC) += snd-soc-omap.o diff --git a/sound/soc/omap/mcpdm.c b/sound/soc/omap/mcpdm.c deleted file mode 100644 index 9746a49..0000000 --- a/sound/soc/omap/mcpdm.c +++ /dev/null @@ -1,472 +0,0 @@ [...] +static int omap_mcpdm_interface_event(struct snd_soc_dapm_widget *w,
struct snd_kcontrol *kcontrol, int event)
+{
- struct omap_mcpdm *mcpdm = w->private_data;
Can't you use struct omap_mcpdm *mcpdm = snd_soc_codec_get_drvdata(codec)?
sorry: struct omap_mcpdm *mcpdm = snd_soc_platform_get_drvdata(w->platform);
This would avoid having to add the private_data field to the widget struct. In it's current form it will only really work, if there is just one instance of the driver using the widget. And if that's the case you can use a global variable directly anyway.
This wont work as the McPDM DAI is neither a codec or a platform driver. It's best to give the widgets some private data since we may have other uses for it too.
oops, sorry. I somehow though this was a platform driver, don't know why. But still the way you use private_data in this driver, you use it to cleverly hide a global variable.
The whole thing is sort of a hack. I mean, if it turns out that we want DAPM on the SoC DAI side, we should probably give it is own DAPM context, instead of using the cards context for this.
A SoC DAI can be represented within DAPM already with the AIF widget, so adding DAI context should be fine.
Liam
- Lars
On Friday 19 August 2011 10:49:08 Lars-Peter Clausen wrote:
sorry: struct omap_mcpdm *mcpdm = snd_soc_platform_get_drvdata(w->platform);
This would avoid having to add the private_data field to the widget struct. In it's current form it will only really work, if there is just one instance of the driver using the widget. And if that's the case you can use a global variable directly anyway.
The other option was to use global variable to get the *mcpdm for the widget. Generally I try to avoid global variables as much as I can, since the use of them looks really hackish. There could be some way to reach the CPU dai via pointers/lookups from the widget, but it does looked really ugly, and I was only half way there.
On Mon, Aug 22, 2011 at 02:34:23PM +0300, Péter Ujfalusi wrote:
On Friday 19 August 2011 10:49:08 Lars-Peter Clausen wrote:
sorry: struct omap_mcpdm *mcpdm = snd_soc_platform_get_drvdata(w->platform);
This would avoid having to add the private_data field to the widget struct. In it's current form it will only really work, if there is just one instance of the driver using the widget. And if that's the case you can use a global variable directly anyway.
The other option was to use global variable to get the *mcpdm for the widget. Generally I try to avoid global variables as much as I can, since the use of them looks really hackish. There could be some way to reach the CPU dai via pointers/lookups from the widget, but it does looked really ugly, and I was only half way there.
The widget will have a DAPM context associated with it, should we perhaps add this facility to the DAPM context where it'd then work for everything? If we're getting fancy the context could look up the CODEC context rather than use a local one if it's for a CODEC.
On 08/22/2011 01:34 PM, Péter Ujfalusi wrote:
On Friday 19 August 2011 10:49:08 Lars-Peter Clausen wrote:
sorry: struct omap_mcpdm *mcpdm = snd_soc_platform_get_drvdata(w->platform);
This would avoid having to add the private_data field to the widget struct. In it's current form it will only really work, if there is just one instance of the driver using the widget. And if that's the case you can use a global variable directly anyway.
The other option was to use global variable to get the *mcpdm for the widget. Generally I try to avoid global variables as much as I can, since the use of them looks really hackish.
omap_mcpdm_widgets is a global variable. You assign to it in asoc_mcpdm_probe and read from it in omap_mcpdm_add_dapm_widget. The fact that you hide your *mcpdm in a void pointer doesn't make it less hackish.
On Monday 22 August 2011 15:39:15 Lars-Peter Clausen wrote:
omap_mcpdm_widgets is a global variable.
Yeah, as most of the snd_soc_dapm_widget.
You assign to it in asoc_mcpdm_probe
Since at compile time I don't have the pointer for the mcpdm (it is allocated earlier in the same function), I need to assign it somewhere.
and read from it in omap_mcpdm_add_dapm_widget.
I don't see any reference to the omap_mcpdm_widgets in there.
The fact that you hide your *mcpdm in a void pointer doesn't make it less hackish.
Well, from that point of view most of the kernel is hackish. We tend to have void pointers for various things, like platform_data, device_data, driver_data, private_data, etc. You see, the point here is that this private_data for the widget can be used for others as well, if needed. It would make no sense to put "struct omap_mcpdm *mcpdm", just because I have this requirement first, does it?
For sure, I could have chosen to create one global pointer for this event handler:
static struct omap_mcpdm *mcpdm_global;
Use the mcpdm_global within omap_mcpdm_interface_event function, and assign it at asoc_mcpdm_probe time.
Would that look better? IMHO it is not.
As Mark suggested, we should have accessor for this, if we are going to have such a thing.
The reason for this (as I described in the commit message): OMAP McPDM has the requirement (along with the twl6040), that we need to stop the cpu dai _after_ the DAC/ADC has been stopped on the codec side. Now, I do not want to hard wire the twl6040, nor the omap-mcpdm with each other. The omap-mcpdm implements the event callback, and the machine driver is responsible to create the DAPM route to make sure we follow the correct sequence. I could as well implemented these things in the machine driver (+global pointer for mcpdm), but that can lead duplicated code in the future (new machine driver, etc).
On 08/23/2011 08:49 AM, Péter Ujfalusi wrote:
On Monday 22 August 2011 15:39:15 Lars-Peter Clausen wrote:
omap_mcpdm_widgets is a global variable.
Yeah, as most of the snd_soc_dapm_widget.
The point is, you use it to pass runtime specific data around, while the others are constant compile time data, which are used as a template.
You assign to it in asoc_mcpdm_probe
Since at compile time I don't have the pointer for the mcpdm (it is allocated earlier in the same function), I need to assign it somewhere.
and read from it in omap_mcpdm_add_dapm_widget.
I don't see any reference to the omap_mcpdm_widgets in there.
+ return snd_soc_dapm_new_controls(dapm, omap_mcpdm_widgets, + ARRAY_SIZE(omap_mcpdm_widgets));
The fact that you hide your *mcpdm in a void pointer doesn't make it less hackish.
Well, from that point of view most of the kernel is hackish. We tend to have void pointers for various things, like platform_data, device_data, driver_data, private_data, etc.
I'm not arguing against such constructs. I'm arguing against your usage of them.
Let me give you an example which is analogous to yours:
static struct platform_device foo;
static void bar_probe(struct platform_device *pdev) { foo.dev.platform_data = ...; }
void bar_some_global_func(void) { platform_device_add(&foo); }
You'll rarely see this in driver code.
If that doesn't convince you, ask yourself what would happen if you had two instances of the mcpdm driver.
You see, the point here is that this private_data for the widget can be used for others as well, if needed. It would make no sense to put "struct omap_mcpdm *mcpdm", just because I have this requirement first, does it?
For sure, I could have chosen to create one global pointer for this event handler:
static struct omap_mcpdm *mcpdm_global;
Use the mcpdm_global within omap_mcpdm_interface_event function, and assign it at asoc_mcpdm_probe time.
Would that look better? IMHO it is not.
Your current solution might look better on the surface, but it is deep ugly on the inside. You've hidden your mcpdm_global in a construct that is normally present in a ASoC driver. You've just slightly changed it in subtitle way, apparently so subtitle you don't even see it yourself.
Hi,
On Tuesday 23 August 2011 12:14:58 Lars-Peter Clausen wrote:
If that doesn't convince you, ask yourself what would happen if you had two instances of the mcpdm driver.
I see what you mean. I guess I was blinded by the fact that we only have one instance of the McPDM driver. Yeah, you are right this was really bad design. I have reworked it for v3.
Thanks, Péter
Am Freitag, den 19.08.2011, 10:41 +0300 schrieb Peter Ujfalusi:
From: Misael Lopez Cruz misael.lopez@ti.com
Reasons for the replacement: The current driver for McPDM was developed to support the legacy mode only. In preparation for the ABE support the current driver stack need the be replaced.
s/need the/needs to/
[…]
Thanks,
Paul
On Fri, Aug 19, 2011 at 10:41:19AM +0300, Peter Ujfalusi wrote:
+/*
- DAPM event function to ensure, that the host side McPDM interface is turned
- off after the codec's DAC.
- */
+static int omap_mcpdm_interface_event(struct snd_soc_dapm_widget *w,
struct snd_kcontrol *kcontrol, int event)
So, the issue here is that the CODEC you're using on your system lacks DAC mute support and doesn't handle the end of the input stream gracefully? This doesn't sound like a McPDM specific issue at all, and I'm slightly surprised we don't run into it more often.
Currently the sequence we use on stream teardown is:
1. Mute. 2. Stop stream. 3. Wait for the DAPM teardown time. 4. Power down.
but it seems like what your CODEC actually wants is:
1. Power down. 2. Stop stream.
which isn't at all unresonable and if the CODEC is actually able to support that mode of operation well then it'll be lower power. This seems like something we should be supporting in the core as I would expect other devices will find it useful, PDM class D speaker drivers being the most obvious example.
I do think it'd be helpful to split this code out as a separate patch as it's the controversial bit...
Hi Mark,
On Saturday 20 August 2011 09:01:14 Mark Brown wrote:
So, the issue here is that the CODEC you're using on your system lacks DAC mute support and doesn't handle the end of the input stream gracefully? This doesn't sound like a McPDM specific issue at all, and I'm slightly surprised we don't run into it more often.
Currently the sequence we use on stream teardown is:
- Mute.
- Stop stream.
- Wait for the DAPM teardown time.
- Power down.
but it seems like what your CODEC actually wants is:
- Power down.
- Stop stream.
Something like that, yes.
which isn't at all unresonable and if the CODEC is actually able to support that mode of operation well then it'll be lower power. This seems like something we should be supporting in the core as I would expect other devices will find it useful, PDM class D speaker drivers being the most obvious example.
That's good, if we have other users with similar requirements towards the sequencing. One thing, which might need special care in the down sequence:
1. Stop platform (DMA) 2. Power down (mostly codec side) 3. Stop cpu dai
I'm not sure about this, but we should not have running DMA after trigger:stop?
I do think it'd be helpful to split this code out as a separate patch as it's the controversial bit...
It is not that easy. There's no incremental way from the old driver to the new one. What I can try however is to write an intermediate driver, which does not have the delayed sequencing. I know that this is a bit problematic, and it is not going to work as good as the driver in this series, but probably it will give the needed separation of the sequencing part. This going to take some time, since I need to - kind of - write a new driver, which is in half way between the two versions ;)
-- Péter
On Tue, Aug 23, 2011 at 10:52:03AM +0300, Péter Ujfalusi wrote:
- Stop platform (DMA)
- Power down (mostly codec side)
- Stop cpu dai
I'm not sure about this, but we should not have running DMA after trigger:stop?
I'd expect that to stop the DMA. I need to think about this properly but I think the main trick is keeping the CPU DAI active for longer than we would normally.
Hi Mark,
On Tuesday 23 August 2011 09:52:03 Ujfalusi, Peter wrote:
I do think it'd be helpful to split this code out as a separate patch as it's the controversial bit...
It is not that easy. There's no incremental way from the old driver to the new one. What I can try however is to write an intermediate driver, which does not have the delayed sequencing. I know that this is a bit problematic, and it is not going to work as good as the driver in this series, but probably it will give the needed separation of the sequencing part. This going to take some time, since I need to - kind of - write a new driver, which is in half way between the two versions ;)
I have looked at the possibility of separating the controversial part from the rest of the change, but it does not look feasible. I would need to write a driver to fill the gap (well a hack driver for the time of one commit), and even in that way the patch which adds the 'controversial' bit is going to contain quite a bit of code, since I need to rework other parts of the driver. I think the improved commit message contains enough information on the change.
Thanks, Péter
participants (5)
-
Lars-Peter Clausen
-
Liam Girdwood
-
Mark Brown
-
Paul Menzel
-
Peter Ujfalusi