[PATCH 0/5] Adding OMAP DMIC driver to kernel

[PATCH 0/5] Adding OMAP DMIC driver to kernel

[David Lambert]

This is a patch series to add the OMAP Digital Microphone
driver for OMAP4.

It includes the driver, a generic DMIC codec, platform devices,
as well as HWMOD entries for OMAP44xx chipsets.

David Lambert (5):
  ASoC: DMIC: Adding the OMAP DMIC driver
  ASoC: DMIC codec: Adding a generic DMIC codec
  ASoC: DMIC: Adding OMAP DMIC driver to build
  OMAP4: hwmod: add entries for DMIC driver
  MAP4: DMIC: Add DMIC codec platform devices

 arch/arm/mach-omap2/board-4430sdp.c        |   11 +
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |   91 +++++
 arch/arm/plat-omap/devices.c               |   35 ++
 arch/arm/plat-omap/include/plat/dmic.h     |   83 ++++
 sound/soc/codecs/Kconfig                   |    3 +
 sound/soc/codecs/Makefile                  |    2 +
 sound/soc/codecs/dmic.c                    |   79 ++++
 sound/soc/omap/Kconfig                     |    5 +
 sound/soc/omap/Makefile                    |    2 +
 sound/soc/omap/omap-dmic.c                 |  596 ++++++++++++++++++++++++++++
 sound/soc/omap/omap-dmic.h                 |   38 ++
 11 files changed, 945 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-omap/include/plat/dmic.h
 create mode 100644 sound/soc/codecs/dmic.c
 create mode 100644 sound/soc/omap/omap-dmic.c
 create mode 100644 sound/soc/omap/omap-dmic.h

[PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[David Lambert]

This patch adds support for the OMAP4 digital microphone DAI.

This DAI can support support recording in 2, 4, or 6 channels

When provided with a 19.2Mhz functional clock, can encode at 96Khz or
192Khz (all
channels must have the same sample rate).

Details of the hardware interface can be found in the OMAP4 TRM in Section 23.7

Signed-off-by: David Lambert <dlambert@ti.com>
---
 arch/arm/plat-omap/include/plat/dmic.h |   83 +++++
 sound/soc/omap/omap-dmic.c             |  596 ++++++++++++++++++++++++++++++++
 sound/soc/omap/omap-dmic.h             |   38 ++
 3 files changed, 717 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-omap/include/plat/dmic.h
 create mode 100644 sound/soc/omap/omap-dmic.c
 create mode 100644 sound/soc/omap/omap-dmic.h

diff --git a/arch/arm/plat-omap/include/plat/dmic.h b/arch/arm/plat-omap/include/plat/dmic.h
new file mode 100644
index 0000000..8a988bf
--- /dev/null
+++ b/arch/arm/plat-omap/include/plat/dmic.h
@@ -0,0 +1,83 @@
+/*
+ * omap-dmic.h  --  OMAP Digital Microphone Controller
+ *
+ * Author: Liam Girdwood <lrg@slimlogic.co.uk>
+ *	   David Lambert <dlambert@ti.com>
+ *	   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.
+ */
+
+#ifndef __ASM_ARCH_OMAP_DMIC_H
+#define __ASM_ARCH_OMAP_DMIC_H
+
+#define OMAP44XX_DMIC_L3_BASE	0x4902e000
+
+#define OMAP_DMIC_REVISION		0x00
+#define OMAP_DMIC_SYSCONFIG		0x10
+#define OMAP_DMIC_IRQSTATUS_RAW		0x24
+#define OMAP_DMIC_IRQSTATUS		0x28
+#define OMAP_DMIC_IRQENABLE_SET		0x2C
+#define OMAP_DMIC_IRQENABLE_CLR		0x30
+#define OMAP_DMIC_IRQWAKE_EN		0x34
+#define OMAP_DMIC_DMAENABLE_SET		0x38
+#define OMAP_DMIC_DMAENABLE_CLR		0x3C
+#define OMAP_DMIC_DMAWAKEEN		0x40
+#define OMAP_DMIC_CTRL			0x44
+#define OMAP_DMIC_DATA			0x48
+#define OMAP_DMIC_FIFO_CTRL		0x4C
+#define OMAP_DMIC_FIFO_DMIC1R_DATA	0x50
+#define OMAP_DMIC_FIFO_DMIC1L_DATA	0x54
+#define OMAP_DMIC_FIFO_DMIC2R_DATA	0x58
+#define OMAP_DMIC_FIFO_DMIC2L_DATA	0x5C
+#define OMAP_DMIC_FIFO_DMIC3R_DATA	0x60
+#define OMAP_DMIC_FIFO_DMIC3L_DATA	0x64
+
+/*
+ * DMIC_IRQ bit fields
+ * IRQSTATUS_RAW, IRQSTATUS, IRQENABLE_SET, IRQENABLE_CLR
+ */
+
+#define OMAP_DMIC_IRQ			(1 << 0)
+#define OMAP_DMIC_IRQ_FULL		(1 << 1)
+#define OMAP_DMIC_IRQ_ALMST_EMPTY	(1 << 2)
+#define OMAP_DMIC_IRQ_EMPTY		(1 << 3)
+#define OMAP_DMIC_IRQ_MASK		0x07
+
+/*
+ * DMIC_DMAENABLE bit fields
+ */
+
+#define OMAP_DMIC_DMA_ENABLE		0x1
+
+/*
+ * DMIC_CTRL bit fields
+ */
+
+#define OMAP_DMIC_UP1_ENABLE		0x0001
+#define OMAP_DMIC_UP2_ENABLE		0x0002
+#define OMAP_DMIC_UP3_ENABLE		0x0004
+#define OMAP_DMIC_FORMAT		0x0008
+#define OMAP_DMIC_POLAR1		0x0010
+#define OMAP_DMIC_POLAR2		0x0020
+#define OMAP_DMIC_POLAR3		0x0040
+#define OMAP_DMIC_POLAR_MASK		0x0070
+#define OMAP_DMIC_CLK_DIV_SHIFT		7
+#define OMAP_DMIC_CLK_DIV_MASK		0x0380
+#define	OMAP_DMIC_RESET			0x0400
+
+#define OMAP_DMIC_ENABLE_MASK		0x007
+
+#define OMAP_DMICOUTFORMAT_LJUST	(0 << 3)
+#define OMAP_DMICOUTFORMAT_RJUST	(1 << 3)
+
+/*
+ * DMIC_FIFO_CTRL bit fields
+ */
+
+#define OMAP_DMIC_THRES_MAX		0xF
+
+
+#endif
diff --git a/sound/soc/omap/omap-dmic.c b/sound/soc/omap/omap-dmic.c
new file mode 100644
index 0000000..6ba05bd
--- /dev/null
+++ b/sound/soc/omap/omap-dmic.c
@@ -0,0 +1,596 @@
+/*
+ * omap-dmic.c  --  OMAP ASoC DMIC DAI driver
+ *
+ * Copyright (C) 2010 Texas Instruments
+ *
+ * Author: Liam Girdwood <lrg@slimlogic.co.uk>
+ *	   David Lambert <dlambert@ti.com>
+ *	   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
+ *
+ */
+
+#undef DEBUG
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/wait.h>
+#include <linux/interrupt.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/slab.h>
+#include <linux/pm_runtime.h>
+
+#include <plat/control.h>
+#include <plat/dma.h>
+#include <plat/dmic.h>
+#include <plat/omap_hwmod.h>
+
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/initval.h>
+#include <sound/soc.h>
+
+#include "omap-dmic.h"
+#include "omap-pcm.h"
+
+#define OMAP_DMIC_RATES		(SNDRV_PCM_RATE_96000 | SNDRV_PCM_RATE_192000)
+#define OMAP_DMIC_FORMATS	SNDRV_PCM_FMTBIT_S32_LE
+
+struct omap_dmic {
+	struct device *dev;
+	void __iomem *io_base;
+	int irq;
+	int clk_freq;
+	int sysclk;
+	int active;
+	spinlock_t lock;
+	struct omap_dmic_link *link;
+};
+
+static struct omap_dmic_link omap_dmic_link = {
+	.irq_mask	= OMAP_DMIC_IRQ_EMPTY | OMAP_DMIC_IRQ_FULL,
+	.threshold	= 2,
+	.format		= OMAP_DMICOUTFORMAT_LJUST,
+	.polar		= OMAP_DMIC_POLAR1 | OMAP_DMIC_POLAR2
+				| OMAP_DMIC_POLAR3,
+};
+
+/*
+ * Stream DMA parameters
+ */
+static struct omap_pcm_dma_data omap_dmic_dai_dma_params = {
+	.name		= "DMIC capture",
+	.data_type	= OMAP_DMA_DATA_TYPE_S32,
+	.sync_mode	= OMAP_DMA_SYNC_PACKET,
+	.packet_size	= 2,
+	.port_addr	= OMAP44XX_DMIC_L3_BASE + OMAP_DMIC_DATA,
+};
+
+
+static inline void omap_dmic_write(struct omap_dmic *dmic,
+		u16 reg, u32 val)
+{
+	__raw_writel(val, dmic->io_base + reg);
+}
+
+static inline int omap_dmic_read(struct omap_dmic *dmic, u16 reg)
+{
+	return __raw_readl(dmic->io_base + reg);
+}
+
+#ifdef DEBUG
+static void omap_dmic_reg_dump(struct omap_dmic *dmic)
+{
+	dev_dbg(dmic->dev, "***********************\n");
+	dev_dbg(dmic->dev, "OMAP_DMIC_IRQSTATUS_RAW:  0x%04x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_IRQSTATUS_RAW));
+	dev_dbg(dmic->dev, "OMAP_DMIC_IRQSTATUS:  0x%04x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_IRQSTATUS));
+	dev_dbg(dmic->dev, "OMAP_DMIC_IRQENABLE_SET:  0x%04x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_IRQENABLE_SET));
+	dev_dbg(dmic->dev, "OMAP_DMIC_IRQENABLE_CLR:  0x%04x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_IRQENABLE_CLR));
+	dev_dbg(dmic->dev, "OMAP_DMIC_IRQWAKE_EN:  0x%04x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_IRQWAKE_EN));
+	dev_dbg(dmic->dev, "OMAP_DMIC_DMAENABLE_SET:  0x%04x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_DMAENABLE_SET));
+	dev_dbg(dmic->dev, "OMAP_DMIC_DMAENABLE_CLR:  0x%04x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_DMAENABLE_CLR));
+	dev_dbg(dmic->dev, "OMAP_DMIC_DMAWAKEEN:  0x%04x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_DMAWAKEEN));
+	dev_dbg(dmic->dev, "OMAP_DMIC_CTRL:  0x%04x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_CTRL));
+	dev_dbg(dmic->dev, "OMAP_DMIC_DATA:  0x%04x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_DATA));
+	dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_CTRL:  0x%04x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_FIFO_CTRL));
+	dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC1R_DATA:  0x%08x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC1R_DATA));
+	dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC1L_DATA:  0x%08x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC1L_DATA));
+	dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC2R_DATA:  0x%08x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC2R_DATA));
+	dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC2L_DATA:  0x%08x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC2L_DATA));
+	dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC3R_DATA:  0x%08x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC3R_DATA));
+	dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC3L_DATA:  0x%08x\n",
+		omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC3L_DATA));
+	dev_dbg(dmic->dev, "***********************\n");
+}
+#else
+static void omap_dmic_reg_dump(struct omap_dmic *dmic) {}
+#endif
+
+/*
+ * Enables the transfer through the DMIC interface
+ */
+static void omap_dmic_start(struct omap_dmic *dmic, int channels)
+{
+	u32 ctrl = omap_dmic_read(dmic, OMAP_DMIC_CTRL);
+	omap_dmic_write(dmic, OMAP_DMIC_CTRL, ctrl | channels);
+
+}
+
+/*
+ * Disables the transfer through the DMIC interface
+ */
+static void omap_dmic_stop(struct omap_dmic *dmic, int channels)
+{
+	u32 ctrl = omap_dmic_read(dmic, OMAP_DMIC_CTRL);
+	omap_dmic_write(dmic, OMAP_DMIC_CTRL, ctrl & ~channels);
+}
+
+static int omap_dmic_set_clkdiv(struct snd_soc_dai *dai,
+				int div_id, int div)
+{
+	struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
+	int ctrl, div_sel = -EINVAL;
+
+	if (div_id != OMAP_DMIC_CLKDIV)
+		return -ENODEV;
+
+	switch (dmic->clk_freq) {
+	case 19200000:
+		if (div == 5)
+			div_sel = 0x1;
+		else if (div == 8)
+			div_sel = 0x0;
+		break;
+	case 24000000:
+		if (div == 10)
+			div_sel = 0x2;
+		break;
+	case 24576000:
+		if (div == 8)
+			div_sel = 0x3;
+		else if (div == 16)
+			div_sel = 0x4;
+		break;
+	case 12000000:
+		if (div == 5)
+			div_sel = 0x5;
+		break;
+	default:
+		dev_err(dai->dev, "invalid freq %d\n", dmic->clk_freq);
+		return -EINVAL;
+	}
+
+	if (div_sel < 0) {
+		dev_err(dai->dev, "divider not supported %d\n", div);
+		return -EINVAL;
+	}
+
+	ctrl = omap_dmic_read(dmic, OMAP_DMIC_CTRL) & ~OMAP_DMIC_CLK_DIV_MASK;
+
+	omap_dmic_write(dmic, OMAP_DMIC_CTRL,
+			ctrl | (div_sel << OMAP_DMIC_CLK_DIV_SHIFT));
+
+	return 0;
+}
+
+/*
+ * Configures DMIC for audio recording.
+ * This function should be called before omap_dmic_start.
+ */
+static int omap_dmic_open(struct omap_dmic *dmic)
+{
+	struct omap_dmic_link *link = dmic->link;
+	u32 ctrl;
+
+	/* Enable irq request generation */
+	omap_dmic_write(dmic, OMAP_DMIC_IRQENABLE_SET,
+			link->irq_mask & OMAP_DMIC_IRQ_MASK);
+
+	/* Configure uplink threshold */
+	if (link->threshold > OMAP_DMIC_THRES_MAX)
+		link->threshold = OMAP_DMIC_THRES_MAX;
+
+	omap_dmic_write(dmic, OMAP_DMIC_FIFO_CTRL, link->threshold);
+
+	/* Configure DMA controller */
+	omap_dmic_write(dmic, OMAP_DMIC_DMAENABLE_SET, OMAP_DMIC_DMA_ENABLE);
+
+	/* Set dmic out format */
+	ctrl = omap_dmic_read(dmic, OMAP_DMIC_CTRL)
+		& ~(OMAP_DMIC_FORMAT | OMAP_DMIC_POLAR_MASK);
+	omap_dmic_write(dmic, OMAP_DMIC_CTRL,
+			ctrl | link->format | link->polar);
+
+	return 0;
+}
+
+/*
+ * Cleans DMIC uplink configuration.
+ * This function should be called when the stream is closed.
+ */
+static int omap_dmic_close(struct omap_dmic *dmic)
+{
+	struct omap_dmic_link *link = dmic->link;
+
+	/* Disable irq request generation */
+	omap_dmic_write(dmic, OMAP_DMIC_IRQENABLE_CLR,
+			link->irq_mask & OMAP_DMIC_IRQ_MASK);
+
+	/* Disable DMA request generation */
+	omap_dmic_write(dmic, OMAP_DMIC_DMAENABLE_CLR, OMAP_DMIC_DMA_ENABLE);
+
+	return 0;
+}
+
+static irqreturn_t omap_dmic_irq_handler(int irq, void *dev_id)
+{
+	struct omap_dmic *dmic = dev_id;
+	u32 irq_status;
+
+	irq_status = omap_dmic_read(dmic, OMAP_DMIC_IRQSTATUS);
+
+	/* Acknowledge irq event */
+	omap_dmic_write(dmic, OMAP_DMIC_IRQSTATUS, irq_status);
+	if (irq_status & OMAP_DMIC_IRQ_FULL)
+		dev_dbg(dmic->dev, "DMIC FIFO error %x\n", irq_status);
+
+	if (irq_status & OMAP_DMIC_IRQ_EMPTY)
+		dev_dbg(dmic->dev, "DMIC FIFO error %x\n", irq_status);
+
+	if (irq_status & OMAP_DMIC_IRQ)
+		dev_dbg(dmic->dev, "DMIC write request\n");
+
+	return IRQ_HANDLED;
+}
+
+static int omap_dmic_dai_startup(struct snd_pcm_substream *substream,
+				  struct snd_soc_dai *dai)
+{
+	struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
+
+	if (!dmic->active++)
+		pm_runtime_get_sync(dmic->dev);
+
+	return 0;
+}
+
+static void omap_dmic_dai_shutdown(struct snd_pcm_substream *substream,
+				    struct snd_soc_dai *dai)
+{
+	struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
+
+	if (!--dmic->active)
+		pm_runtime_put_sync(dmic->dev);
+}
+
+static int omap_dmic_dai_hw_params(struct snd_pcm_substream *substream,
+				    struct snd_pcm_hw_params *params,
+				    struct snd_soc_dai *dai)
+{
+	struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
+	struct omap_dmic_link *link = dmic->link;
+	int channels, rate, div;
+	int ret = 0;
+
+	channels = params_channels(params);
+	switch (channels) {
+	case 6:
+		link->channels = OMAP_DMIC_UP1_ENABLE | OMAP_DMIC_UP2_ENABLE
+			 | OMAP_DMIC_UP3_ENABLE;
+		break;
+	case 4:
+		link->channels = OMAP_DMIC_UP1_ENABLE | OMAP_DMIC_UP2_ENABLE;
+		break;
+	case 2:
+		link->channels = OMAP_DMIC_UP1_ENABLE;
+		break;
+	default:
+		dev_err(dmic->dev, "invalid number of channels\n");
+		return -EINVAL;
+	}
+
+	rate = params_rate(params);
+	switch (rate) {
+	case 192000:
+		div = 5;
+		break;
+	default:
+		div = 8;
+	}
+
+	omap_dmic_set_clkdiv(dai, OMAP_DMIC_CLKDIV, div);
+
+	omap_dmic_dai_dma_params.packet_size = link->threshold * channels;
+	snd_soc_dai_set_dma_data(dai, substream, &omap_dmic_dai_dma_params);
+
+	if (dmic->active == 1)
+		ret = omap_dmic_open(dmic);
+
+	return ret;
+}
+
+static int omap_dmic_dai_hw_free(struct snd_pcm_substream *substream,
+				  struct snd_soc_dai *dai)
+{
+	struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
+	struct omap_dmic_link *link = dmic->link;
+	int ret = 0;
+
+	if (dmic->active == 1) {
+		ret = omap_dmic_close(dmic);
+		link->channels = 0;
+	}
+
+	return ret;
+}
+
+static int omap_dmic_dai_trigger(struct snd_pcm_substream *substream,
+				  int cmd, struct snd_soc_dai *dai)
+{
+	struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
+	int dmic_id = dmic->link->channels;
+
+	omap_dmic_reg_dump(dmic);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		omap_dmic_start(dmic, dmic_id);
+		break;
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		break;
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+		omap_dmic_stop(dmic, dmic_id);
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int omap_dmic_set_dai_sysclk(struct snd_soc_dai *dai,
+				    int clk_id, unsigned int freq,
+				    int dir)
+{
+	struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
+	struct clk *dmic_clk, *parent_clk;
+	int ret = 0;
+
+	dmic_clk = clk_get(NULL, "dmic_fck");
+	if (IS_ERR(dmic_clk))
+		return -ENODEV;
+
+	switch (clk_id) {
+	case OMAP_DMIC_SYSCLK_PAD_CLKS:
+		parent_clk = clk_get(NULL, "pad_clks_ck");
+		if (IS_ERR(parent_clk)) {
+			ret = -ENODEV;
+			goto err_par;
+		}
+		break;
+	case OMAP_DMIC_SYSCLK_SLIMBLUS_CLKS:
+		parent_clk = clk_get(NULL, "slimbus_clk");
+		if (IS_ERR(parent_clk)) {
+			ret = -ENODEV;
+			goto err_par;
+		}
+		break;
+	case OMAP_DMIC_SYSCLK_SYNC_MUX_CLKS:
+		parent_clk = clk_get(NULL, "dmic_sync_mux_ck");
+		if (IS_ERR(parent_clk)) {
+			ret = -ENODEV;
+			goto err_par;
+		}
+		break;
+	default:
+		dev_err(dai->dev, "clk_id not supported %d\n", clk_id);
+		ret = -EINVAL;
+		goto err_par;
+	}
+
+	switch (freq) {
+	case 19200000:
+	case 24000000:
+	case 24576000:
+	case 12000000:
+		dmic->clk_freq = freq;
+		break;
+	default:
+		dev_err(dai->dev, "clk freq not supported %d\n", freq);
+		ret = -EINVAL;
+		goto err_freq;
+	}
+
+	if (dmic->sysclk != clk_id) {
+		/* reparent not allowed if a stream is ongoing */
+		if (dmic->active > 1) {
+			ret = -EBUSY;
+			goto err_freq;
+		}
+
+		/* disable clock while reparenting */
+		if (dmic->active == 1)
+			pm_runtime_put_sync(dmic->dev);
+
+		ret = clk_set_parent(dmic_clk, parent_clk);
+
+		if (dmic->active == 1)
+			pm_runtime_get_sync(dmic->dev);
+
+		dmic->sysclk = clk_id;
+	}
+
+err_freq:
+	clk_put(parent_clk);
+err_par:
+	clk_put(dmic_clk);
+
+	return ret;
+}
+
+static struct snd_soc_dai_ops omap_dmic_dai_ops = {
+	.startup	= omap_dmic_dai_startup,
+	.shutdown	= omap_dmic_dai_shutdown,
+	.hw_params	= omap_dmic_dai_hw_params,
+	.trigger	= omap_dmic_dai_trigger,
+	.hw_free	= omap_dmic_dai_hw_free,
+	.set_sysclk	= omap_dmic_set_dai_sysclk,
+	.set_clkdiv	= omap_dmic_set_clkdiv,
+};
+
+static struct snd_soc_dai_driver omap_dmic_dai = {
+
+	.name = "omap-dmic-dai-0",
+	.capture = {
+		.channels_min = 2,
+		.channels_max = 6,
+		.rates = OMAP_DMIC_RATES,
+		.formats = OMAP_DMIC_FORMATS,
+	},
+	.ops = &omap_dmic_dai_ops,
+};
+
+static __devinit int asoc_dmic_probe(struct platform_device *pdev)
+{
+	struct omap_dmic *dmic;
+	struct resource *res;
+	int ret;
+
+	dmic = kzalloc(sizeof(struct omap_dmic), GFP_KERNEL);
+	if (!dmic)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, dmic);
+	dmic->dev = &pdev->dev;
+	dmic->link = &omap_dmic_link;
+	dmic->sysclk = OMAP_DMIC_SYSCLK_SYNC_MUX_CLKS;
+
+	spin_lock_init(&dmic->lock);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(dmic->dev, "invalid memory resource\n");
+		ret = -ENODEV;
+		goto err_res;
+	}
+
+	dmic->io_base = ioremap(res->start, resource_size(res));
+	if (!dmic->io_base) {
+		ret = -ENOMEM;
+		goto err_res;
+	}
+
+	dmic->irq = platform_get_irq(pdev, 0);
+	if (dmic->irq < 0) {
+		ret = dmic->irq;
+		goto err_irq;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_DMA, 0);
+	if (!res) {
+		dev_err(dmic->dev, "invalid dma resource\n");
+		ret = -ENODEV;
+		goto err_irq;
+	}
+	omap_dmic_dai_dma_params.dma_req = res->start;
+
+	pm_runtime_enable(dmic->dev);
+
+	/* Disable lines while request is ongoing */
+	omap_dmic_write(dmic, OMAP_DMIC_CTRL, 0x00);
+
+	ret = request_threaded_irq(dmic->irq, NULL, omap_dmic_irq_handler,
+				   IRQF_ONESHOT, "DMIC", (void *)dmic);
+	if (ret) {
+		dev_err(dmic->dev, "irq request failed\n");
+		goto err_irq;
+	}
+
+	ret = snd_soc_register_dais(&pdev->dev, &omap_dmic_dai, 1);
+	if (ret)
+		goto err_dai;
+
+	return 0;
+
+err_dai:
+	free_irq(dmic->irq, dmic);
+err_irq:
+	iounmap(dmic->io_base);
+err_res:
+	kfree(dmic);
+	return ret;
+}
+
+static int __devexit asoc_dmic_remove(struct platform_device *pdev)
+{
+	struct omap_dmic *dmic = platform_get_drvdata(pdev);
+
+	snd_soc_unregister_dai(&pdev->dev);
+	free_irq(dmic->irq, dmic);
+	iounmap(dmic->io_base);
+	kfree(dmic);
+
+	return 0;
+}
+
+static struct platform_driver asoc_dmic_driver = {
+	.driver = {
+		.name = "omap-dmic-dai",
+		.owner = THIS_MODULE,
+	},
+	.probe = asoc_dmic_probe,
+	.remove = __devexit_p(asoc_dmic_remove),
+};
+
+static int __init snd_omap_dmic_init(void)
+{
+	return platform_driver_register(&asoc_dmic_driver);
+}
+module_init(snd_omap_dmic_init);
+
+static void __exit snd_omap_dmic_exit(void)
+{
+	platform_driver_unregister(&asoc_dmic_driver);
+}
+module_exit(snd_omap_dmic_exit);
+
+MODULE_AUTHOR("David Lambert <dlambert@ti.com>");
+MODULE_DESCRIPTION("OMAP DMIC SoC Interface");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/omap/omap-dmic.h b/sound/soc/omap/omap-dmic.h
new file mode 100644
index 0000000..6a95c44
--- /dev/null
+++ b/sound/soc/omap/omap-dmic.h
@@ -0,0 +1,38 @@
+/*
+ * omap-dmic.h  --  OMAP Digital Microphone Controller
+ *
+ * Author: Liam Girdwood <lrg@slimlogic.co.uk>
+ *	   David Lambert <dlambert@ti.com>
+ *	   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.
+ */
+
+#ifndef _OMAP_DMIC_H
+#define _OMAP_DMIC_H
+
+#include <sound/soc.h>
+
+
+enum omap_dmic_clk {
+	OMAP_DMIC_SYSCLK_PAD_CLKS,		/* PAD_CLKS */
+	OMAP_DMIC_SYSCLK_SLIMBLUS_CLKS,		/* SLIMBUS_CLK */
+	OMAP_DMIC_SYSCLK_SYNC_MUX_CLKS,		/* DMIC_SYNC_MUX_CLK */
+};
+
+/* DMIC dividers */
+enum omap_dmic_div {
+	OMAP_DMIC_CLKDIV,
+};
+
+struct omap_dmic_link {
+	int irq_mask;
+	int threshold;
+	int format;
+	int channels;
+	int polar;
+};
+
+#endif
-- 
1.7.0.4

Re: [PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[Felipe Balbi]

Hi,

On Mon, Dec 27, 2010 at 10:17:02PM -0600, David Lambert wrote:
>This patch adds support for the OMAP4 digital microphone DAI.
>
>This DAI can support support recording in 2, 4, or 6 channels
>
>When provided with a 19.2Mhz functional clock, can encode at 96Khz or
>192Khz (all
>channels must have the same sample rate).
>
>Details of the hardware interface can be found in the OMAP4 TRM in Section 23.7
>
>Signed-off-by: David Lambert <dlambert@ti.com>
>---
> arch/arm/plat-omap/include/plat/dmic.h |   83 +++++
> sound/soc/omap/omap-dmic.c             |  596 ++++++++++++++++++++++++++++++++
> sound/soc/omap/omap-dmic.h             |   38 ++
> 3 files changed, 717 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/plat-omap/include/plat/dmic.h
> create mode 100644 sound/soc/omap/omap-dmic.c
> create mode 100644 sound/soc/omap/omap-dmic.h
>
>diff --git a/arch/arm/plat-omap/include/plat/dmic.h b/arch/arm/plat-omap/include/plat/dmic.h
>new file mode 100644
>index 0000000..8a988bf
>--- /dev/null
>+++ b/arch/arm/plat-omap/include/plat/dmic.h
>@@ -0,0 +1,83 @@
>+/*
>+ * omap-dmic.h  --  OMAP Digital Microphone Controller
>+ *
>+ * Author: Liam Girdwood <lrg@slimlogic.co.uk>
>+ *	   David Lambert <dlambert@ti.com>
>+ *	   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.
>+ */
>+
>+#ifndef __ASM_ARCH_OMAP_DMIC_H
>+#define __ASM_ARCH_OMAP_DMIC_H
>+
>+#define OMAP44XX_DMIC_L3_BASE	0x4902e000
>+
>+#define OMAP_DMIC_REVISION		0x00
>+#define OMAP_DMIC_SYSCONFIG		0x10
>+#define OMAP_DMIC_IRQSTATUS_RAW		0x24
>+#define OMAP_DMIC_IRQSTATUS		0x28
>+#define OMAP_DMIC_IRQENABLE_SET		0x2C
>+#define OMAP_DMIC_IRQENABLE_CLR		0x30
>+#define OMAP_DMIC_IRQWAKE_EN		0x34
>+#define OMAP_DMIC_DMAENABLE_SET		0x38
>+#define OMAP_DMIC_DMAENABLE_CLR		0x3C
>+#define OMAP_DMIC_DMAWAKEEN		0x40
>+#define OMAP_DMIC_CTRL			0x44
>+#define OMAP_DMIC_DATA			0x48
>+#define OMAP_DMIC_FIFO_CTRL		0x4C
>+#define OMAP_DMIC_FIFO_DMIC1R_DATA	0x50
>+#define OMAP_DMIC_FIFO_DMIC1L_DATA	0x54
>+#define OMAP_DMIC_FIFO_DMIC2R_DATA	0x58
>+#define OMAP_DMIC_FIFO_DMIC2L_DATA	0x5C
>+#define OMAP_DMIC_FIFO_DMIC3R_DATA	0x60
>+#define OMAP_DMIC_FIFO_DMIC3L_DATA	0x64
>+
>+/*
>+ * DMIC_IRQ bit fields
>+ * IRQSTATUS_RAW, IRQSTATUS, IRQENABLE_SET, IRQENABLE_CLR
>+ */
>+
>+#define OMAP_DMIC_IRQ			(1 << 0)
>+#define OMAP_DMIC_IRQ_FULL		(1 << 1)
>+#define OMAP_DMIC_IRQ_ALMST_EMPTY	(1 << 2)
>+#define OMAP_DMIC_IRQ_EMPTY		(1 << 3)
>+#define OMAP_DMIC_IRQ_MASK		0x07
>+
>+/*
>+ * DMIC_DMAENABLE bit fields
>+ */
>+
>+#define OMAP_DMIC_DMA_ENABLE		0x1
>+
>+/*
>+ * DMIC_CTRL bit fields
>+ */
>+
>+#define OMAP_DMIC_UP1_ENABLE		0x0001
>+#define OMAP_DMIC_UP2_ENABLE		0x0002
>+#define OMAP_DMIC_UP3_ENABLE		0x0004
>+#define OMAP_DMIC_FORMAT		0x0008
>+#define OMAP_DMIC_POLAR1		0x0010
>+#define OMAP_DMIC_POLAR2		0x0020
>+#define OMAP_DMIC_POLAR3		0x0040
>+#define OMAP_DMIC_POLAR_MASK		0x0070
>+#define OMAP_DMIC_CLK_DIV_SHIFT		7
>+#define OMAP_DMIC_CLK_DIV_MASK		0x0380
>+#define	OMAP_DMIC_RESET			0x0400
>+
>+#define OMAP_DMIC_ENABLE_MASK		0x007
>+
>+#define OMAP_DMICOUTFORMAT_LJUST	(0 << 3)
>+#define OMAP_DMICOUTFORMAT_RJUST	(1 << 3)
>+
>+/*
>+ * DMIC_FIFO_CTRL bit fields
>+ */
>+
>+#define OMAP_DMIC_THRES_MAX		0xF
>+
>+

one blank line only. BTW, are these used anywwhere outside the dmic.c
driver ? If not, it's better to move the definitions there.

>+#endif
>diff --git a/sound/soc/omap/omap-dmic.c b/sound/soc/omap/omap-dmic.c
>new file mode 100644
>index 0000000..6ba05bd
>--- /dev/null
>+++ b/sound/soc/omap/omap-dmic.c
>@@ -0,0 +1,596 @@
>+/*
>+ * omap-dmic.c  --  OMAP ASoC DMIC DAI driver
>+ *
>+ * Copyright (C) 2010 Texas Instruments
>+ *
>+ * Author: Liam Girdwood <lrg@slimlogic.co.uk>
>+ *	   David Lambert <dlambert@ti.com>
>+ *	   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
>+ *
>+ */
>+
>+#undef DEBUG
>+
>+#include <linux/init.h>
>+#include <linux/module.h>
>+#include <linux/platform_device.h>
>+#include <linux/wait.h>
>+#include <linux/interrupt.h>
>+#include <linux/err.h>
>+#include <linux/clk.h>
>+#include <linux/delay.h>
>+#include <linux/io.h>
>+#include <linux/irq.h>
>+#include <linux/slab.h>
>+#include <linux/pm_runtime.h>
>+
>+#include <plat/control.h>
>+#include <plat/dma.h>
>+#include <plat/dmic.h>
>+#include <plat/omap_hwmod.h>

I'm not sure drivers should be including these (besides dmic.h which you
added). plat/control.h doesn't even exist anymore and hwmod is supposed
to live under mach-omap*/ only.

>+#include <sound/core.h>
>+#include <sound/pcm.h>
>+#include <sound/pcm_params.h>
>+#include <sound/initval.h>
>+#include <sound/soc.h>
>+
>+#include "omap-dmic.h"
>+#include "omap-pcm.h"

These little guys here look rather small to deserve being split to
separate files. Also, it looks like they're only used by this driver
(??) so it's better to move the definitions to this C source file
instead.

>+static struct omap_dmic_link omap_dmic_link = {

const ?

>+	.irq_mask	= OMAP_DMIC_IRQ_EMPTY | OMAP_DMIC_IRQ_FULL,
>+	.threshold	= 2,
>+	.format		= OMAP_DMICOUTFORMAT_LJUST,
>+	.polar		= OMAP_DMIC_POLAR1 | OMAP_DMIC_POLAR2
>+				| OMAP_DMIC_POLAR3,
>+};
>+
>+/*
>+ * Stream DMA parameters
>+ */
>+static struct omap_pcm_dma_data omap_dmic_dai_dma_params = {

const ?

>+	.name		= "DMIC capture",
>+	.data_type	= OMAP_DMA_DATA_TYPE_S32,
>+	.sync_mode	= OMAP_DMA_SYNC_PACKET,
>+	.packet_size	= 2,
>+	.port_addr	= OMAP44XX_DMIC_L3_BASE + OMAP_DMIC_DATA,
>+};
>+
>+

one blank line only.

>+#ifdef DEBUG
>+static void omap_dmic_reg_dump(struct omap_dmic *dmic)
>+{
>+	dev_dbg(dmic->dev, "***********************\n");
>+	dev_dbg(dmic->dev, "OMAP_DMIC_IRQSTATUS_RAW:  0x%04x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_IRQSTATUS_RAW));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_IRQSTATUS:  0x%04x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_IRQSTATUS));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_IRQENABLE_SET:  0x%04x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_IRQENABLE_SET));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_IRQENABLE_CLR:  0x%04x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_IRQENABLE_CLR));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_IRQWAKE_EN:  0x%04x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_IRQWAKE_EN));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_DMAENABLE_SET:  0x%04x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_DMAENABLE_SET));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_DMAENABLE_CLR:  0x%04x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_DMAENABLE_CLR));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_DMAWAKEEN:  0x%04x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_DMAWAKEEN));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_CTRL:  0x%04x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_CTRL));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_DATA:  0x%04x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_DATA));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_CTRL:  0x%04x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_FIFO_CTRL));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC1R_DATA:  0x%08x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC1R_DATA));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC1L_DATA:  0x%08x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC1L_DATA));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC2R_DATA:  0x%08x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC2R_DATA));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC2L_DATA:  0x%08x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC2L_DATA));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC3R_DATA:  0x%08x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC3R_DATA));
>+	dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC3L_DATA:  0x%08x\n",
>+		omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC3L_DATA));
>+	dev_dbg(dmic->dev, "***********************\n");
>+}
>+#else
>+static void omap_dmic_reg_dump(struct omap_dmic *dmic) {}
>+#endif

Would be better to make a debugfs layer ??

>+static irqreturn_t omap_dmic_irq_handler(int irq, void *dev_id)
>+{
>+	struct omap_dmic *dmic = dev_id;
>+	u32 irq_status;
>+
>+	irq_status = omap_dmic_read(dmic, OMAP_DMIC_IRQSTATUS);
>+
>+	/* Acknowledge irq event */
>+	omap_dmic_write(dmic, OMAP_DMIC_IRQSTATUS, irq_status);
>+	if (irq_status & OMAP_DMIC_IRQ_FULL)
>+		dev_dbg(dmic->dev, "DMIC FIFO error %x\n", irq_status);
>+
>+	if (irq_status & OMAP_DMIC_IRQ_EMPTY)
>+		dev_dbg(dmic->dev, "DMIC FIFO error %x\n", irq_status);
>+
>+	if (irq_status & OMAP_DMIC_IRQ)
>+		dev_dbg(dmic->dev, "DMIC write request\n");

no locking needed ??

>+static int omap_dmic_dai_startup(struct snd_pcm_substream *substream,
>+				  struct snd_soc_dai *dai)
>+{
>+	struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
>+
>+	if (!dmic->active++)
>+		pm_runtime_get_sync(dmic->dev);
>+
>+	return 0;
>+}
>+
>+static void omap_dmic_dai_shutdown(struct snd_pcm_substream *substream,
>+				    struct snd_soc_dai *dai)
>+{
>+	struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
>+
>+	if (!--dmic->active)
>+		pm_runtime_put_sync(dmic->dev);

I could be wrong but I believe pm_runtime implementation on OMAP has
its own refcounting, so you could drop the need for dmic->active.

>+static struct snd_soc_dai_ops omap_dmic_dai_ops = {

should this be const ?

>+static struct snd_soc_dai_driver omap_dmic_dai = {

this too ??

>+static __devinit int asoc_dmic_probe(struct platform_device *pdev)
>+{
>+	struct omap_dmic *dmic;
>+	struct resource *res;
>+	int ret;
>+
>+	dmic = kzalloc(sizeof(struct omap_dmic), GFP_KERNEL);
>+	if (!dmic)
>+		return -ENOMEM;
>+
>+	platform_set_drvdata(pdev, dmic);
>+	dmic->dev = &pdev->dev;
>+	dmic->link = &omap_dmic_link;
>+	dmic->sysclk = OMAP_DMIC_SYSCLK_SYNC_MUX_CLKS;
>+
>+	spin_lock_init(&dmic->lock);
>+
>+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>+	if (!res) {
>+		dev_err(dmic->dev, "invalid memory resource\n");
>+		ret = -ENODEV;
>+		goto err_res;
>+	}
>+
>+	dmic->io_base = ioremap(res->start, resource_size(res));
>+	if (!dmic->io_base) {
>+		ret = -ENOMEM;
>+		goto err_res;
>+	}
>+
>+	dmic->irq = platform_get_irq(pdev, 0);
>+	if (dmic->irq < 0) {
>+		ret = dmic->irq;
>+		goto err_irq;
>+	}
>+
>+	res = platform_get_resource(pdev, IORESOURCE_DMA, 0);
>+	if (!res) {
>+		dev_err(dmic->dev, "invalid dma resource\n");
>+		ret = -ENODEV;
>+		goto err_irq;
>+	}
>+	omap_dmic_dai_dma_params.dma_req = res->start;
>+
>+	pm_runtime_enable(dmic->dev);
>+
>+	/* Disable lines while request is ongoing */
>+	omap_dmic_write(dmic, OMAP_DMIC_CTRL, 0x00);
>+
>+	ret = request_threaded_irq(dmic->irq, NULL, omap_dmic_irq_handler,
>+				   IRQF_ONESHOT, "DMIC", (void *)dmic);

Does this need to be threaded ? Doesn't look like. Also, you don't need
the (void *) cast.

>+static int __devexit asoc_dmic_remove(struct platform_device *pdev)
>+{
>+	struct omap_dmic *dmic = platform_get_drvdata(pdev);
>+
>+	snd_soc_unregister_dai(&pdev->dev);
>+	free_irq(dmic->irq, dmic);
>+	iounmap(dmic->io_base);
>+	kfree(dmic);

pm_runtime_disable() ??

-- 
balbi

Re: [PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[Lambert, David]

On Tue, Dec 28, 2010 at 5:14 AM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Mon, Dec 27, 2010 at 10:17:02PM -0600, David Lambert wrote:
>>
>> This patch adds support for the OMAP4 digital microphone DAI.
>>
>> This DAI can support support recording in 2, 4, or 6 channels
>>
>> When provided with a 19.2Mhz functional clock, can encode at 96Khz or
>> 192Khz (all
>> channels must have the same sample rate).
>>
>> Details of the hardware interface can be found in the OMAP4 TRM in Section
>> 23.7
>>
>> Signed-off-by: David Lambert <dlambert@ti.com>
>> ---
>> arch/arm/plat-omap/include/plat/dmic.h |   83 +++++
>> sound/soc/omap/omap-dmic.c             |  596
>> ++++++++++++++++++++++++++++++++
>> sound/soc/omap/omap-dmic.h             |   38 ++
>> 3 files changed, 717 insertions(+), 0 deletions(-)
>> create mode 100644 arch/arm/plat-omap/include/plat/dmic.h
>> create mode 100644 sound/soc/omap/omap-dmic.c
>> create mode 100644 sound/soc/omap/omap-dmic.h
>>
>> diff --git a/arch/arm/plat-omap/include/plat/dmic.h
>> b/arch/arm/plat-omap/include/plat/dmic.h
>> new file mode 100644
>> index 0000000..8a988bf
>> --- /dev/null
>> +++ b/arch/arm/plat-omap/include/plat/dmic.h
>> @@ -0,0 +1,83 @@
>> +/*
>> + * omap-dmic.h  --  OMAP Digital Microphone Controller
>> + *
>> + * Author: Liam Girdwood <lrg@slimlogic.co.uk>
>> + *        David Lambert <dlambert@ti.com>
>> + *        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.
>> + */
>> +
>> +#ifndef __ASM_ARCH_OMAP_DMIC_H
>> +#define __ASM_ARCH_OMAP_DMIC_H
>> +
>> +#define OMAP44XX_DMIC_L3_BASE  0x4902e000
>> +
>> +#define OMAP_DMIC_REVISION             0x00
>> +#define OMAP_DMIC_SYSCONFIG            0x10
>> +#define OMAP_DMIC_IRQSTATUS_RAW                0x24
>> +#define OMAP_DMIC_IRQSTATUS            0x28
>> +#define OMAP_DMIC_IRQENABLE_SET                0x2C
>> +#define OMAP_DMIC_IRQENABLE_CLR                0x30
>> +#define OMAP_DMIC_IRQWAKE_EN           0x34
>> +#define OMAP_DMIC_DMAENABLE_SET                0x38
>> +#define OMAP_DMIC_DMAENABLE_CLR                0x3C
>> +#define OMAP_DMIC_DMAWAKEEN            0x40
>> +#define OMAP_DMIC_CTRL                 0x44
>> +#define OMAP_DMIC_DATA                 0x48
>> +#define OMAP_DMIC_FIFO_CTRL            0x4C
>> +#define OMAP_DMIC_FIFO_DMIC1R_DATA     0x50
>> +#define OMAP_DMIC_FIFO_DMIC1L_DATA     0x54
>> +#define OMAP_DMIC_FIFO_DMIC2R_DATA     0x58
>> +#define OMAP_DMIC_FIFO_DMIC2L_DATA     0x5C
>> +#define OMAP_DMIC_FIFO_DMIC3R_DATA     0x60
>> +#define OMAP_DMIC_FIFO_DMIC3L_DATA     0x64
>> +
>> +/*
>> + * DMIC_IRQ bit fields
>> + * IRQSTATUS_RAW, IRQSTATUS, IRQENABLE_SET, IRQENABLE_CLR
>> + */
>> +
>> +#define OMAP_DMIC_IRQ                  (1 << 0)
>> +#define OMAP_DMIC_IRQ_FULL             (1 << 1)
>> +#define OMAP_DMIC_IRQ_ALMST_EMPTY      (1 << 2)
>> +#define OMAP_DMIC_IRQ_EMPTY            (1 << 3)
>> +#define OMAP_DMIC_IRQ_MASK             0x07
>> +
>> +/*
>> + * DMIC_DMAENABLE bit fields
>> + */
>> +
>> +#define OMAP_DMIC_DMA_ENABLE           0x1
>> +
>> +/*
>> + * DMIC_CTRL bit fields
>> + */
>> +
>> +#define OMAP_DMIC_UP1_ENABLE           0x0001
>> +#define OMAP_DMIC_UP2_ENABLE           0x0002
>> +#define OMAP_DMIC_UP3_ENABLE           0x0004
>> +#define OMAP_DMIC_FORMAT               0x0008
>> +#define OMAP_DMIC_POLAR1               0x0010
>> +#define OMAP_DMIC_POLAR2               0x0020
>> +#define OMAP_DMIC_POLAR3               0x0040
>> +#define OMAP_DMIC_POLAR_MASK           0x0070
>> +#define OMAP_DMIC_CLK_DIV_SHIFT                7
>> +#define OMAP_DMIC_CLK_DIV_MASK         0x0380
>> +#define        OMAP_DMIC_RESET                 0x0400
>> +
>> +#define OMAP_DMIC_ENABLE_MASK          0x007
>> +
>> +#define OMAP_DMICOUTFORMAT_LJUST       (0 << 3)
>> +#define OMAP_DMICOUTFORMAT_RJUST       (1 << 3)
>> +
>> +/*
>> + * DMIC_FIFO_CTRL bit fields
>> + */
>> +
>> +#define OMAP_DMIC_THRES_MAX            0xF
>> +
>> +
>
> one blank line only. BTW, are these used anywwhere outside the dmic.c
> driver ? If not, it's better to move the definitions there.
>

They were originally in the omap-dmic.h header, but it was suggested
that we move
them to a platform header so that the driver could be more
architecture independent
and we could put the platform specific details here.  I'm OK putting
them just about
anywhere, as long as we have consensus.

>> +#endif
>> diff --git a/sound/soc/omap/omap-dmic.c b/sound/soc/omap/omap-dmic.c
>> new file mode 100644
>> index 0000000..6ba05bd
>> --- /dev/null
>> +++ b/sound/soc/omap/omap-dmic.c
>> @@ -0,0 +1,596 @@
>> +/*
>> + * omap-dmic.c  --  OMAP ASoC DMIC DAI driver
>> + *
>> + * Copyright (C) 2010 Texas Instruments
>> + *
>> + * Author: Liam Girdwood <lrg@slimlogic.co.uk>
>> + *        David Lambert <dlambert@ti.com>
>> + *        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
>> + *
>> + */
>> +
>> +#undef DEBUG
>> +
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/wait.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/err.h>
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/io.h>
>> +#include <linux/irq.h>
>> +#include <linux/slab.h>
>> +#include <linux/pm_runtime.h>
>> +
>> +#include <plat/control.h>
>> +#include <plat/dma.h>
>> +#include <plat/dmic.h>
>> +#include <plat/omap_hwmod.h>
>
> I'm not sure drivers should be including these (besides dmic.h which you
> added). plat/control.h doesn't even exist anymore and hwmod is supposed
> to live under mach-omap*/ only.
>
>> +#include <sound/core.h>
>> +#include <sound/pcm.h>
>> +#include <sound/pcm_params.h>
>> +#include <sound/initval.h>
>> +#include <sound/soc.h>
>> +
>> +#include "omap-dmic.h"
>> +#include "omap-pcm.h"
>
> These little guys here look rather small to deserve being split to
> separate files. Also, it looks like they're only used by this driver
> (??) so it's better to move the definitions to this C source file
> instead.

Agreed, see comment above about the location of the macros for the
registers.  If we agree it belongs in the platform header,  we can collapse
the content of omap-dmic.h in to the C source file.  If we don't think they
belong in a platform header, they can come back here and then it won't
be quite so little.

>
>> +static struct omap_dmic_link omap_dmic_link = {
>
> const ?
>
>> +       .irq_mask       = OMAP_DMIC_IRQ_EMPTY | OMAP_DMIC_IRQ_FULL,
>> +       .threshold      = 2,
>> +       .format         = OMAP_DMICOUTFORMAT_LJUST,
>> +       .polar          = OMAP_DMIC_POLAR1 | OMAP_DMIC_POLAR2
>> +                               | OMAP_DMIC_POLAR3,
>> +};
>> +
>> +/*
>> + * Stream DMA parameters
>> + */
>> +static struct omap_pcm_dma_data omap_dmic_dai_dma_params = {
>
> const ?
>
>> +       .name           = "DMIC capture",
>> +       .data_type      = OMAP_DMA_DATA_TYPE_S32,
>> +       .sync_mode      = OMAP_DMA_SYNC_PACKET,
>> +       .packet_size    = 2,
>> +       .port_addr      = OMAP44XX_DMIC_L3_BASE + OMAP_DMIC_DATA,
>> +};
>> +
>> +
>
> one blank line only.
>
>> +#ifdef DEBUG
>> +static void omap_dmic_reg_dump(struct omap_dmic *dmic)
>> +{
>> +       dev_dbg(dmic->dev, "***********************\n");
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_IRQSTATUS_RAW:  0x%04x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_IRQSTATUS_RAW));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_IRQSTATUS:  0x%04x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_IRQSTATUS));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_IRQENABLE_SET:  0x%04x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_IRQENABLE_SET));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_IRQENABLE_CLR:  0x%04x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_IRQENABLE_CLR));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_IRQWAKE_EN:  0x%04x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_IRQWAKE_EN));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_DMAENABLE_SET:  0x%04x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_DMAENABLE_SET));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_DMAENABLE_CLR:  0x%04x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_DMAENABLE_CLR));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_DMAWAKEEN:  0x%04x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_DMAWAKEEN));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_CTRL:  0x%04x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_CTRL));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_DATA:  0x%04x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_DATA));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_CTRL:  0x%04x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_FIFO_CTRL));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC1R_DATA:  0x%08x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC1R_DATA));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC1L_DATA:  0x%08x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC1L_DATA));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC2R_DATA:  0x%08x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC2R_DATA));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC2L_DATA:  0x%08x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC2L_DATA));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC3R_DATA:  0x%08x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC3R_DATA));
>> +       dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC3L_DATA:  0x%08x\n",
>> +               omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC3L_DATA));
>> +       dev_dbg(dmic->dev, "***********************\n");
>> +}
>> +#else
>> +static void omap_dmic_reg_dump(struct omap_dmic *dmic) {}
>> +#endif
>
> Would be better to make a debugfs layer ??

I'll look in to what it would take to do this.  Could this be a
feature to add later?

>
>> +static irqreturn_t omap_dmic_irq_handler(int irq, void *dev_id)
>> +{
>> +       struct omap_dmic *dmic = dev_id;
>> +       u32 irq_status;
>> +
>> +       irq_status = omap_dmic_read(dmic, OMAP_DMIC_IRQSTATUS);
>> +
>> +       /* Acknowledge irq event */
>> +       omap_dmic_write(dmic, OMAP_DMIC_IRQSTATUS, irq_status);
>> +       if (irq_status & OMAP_DMIC_IRQ_FULL)
>> +               dev_dbg(dmic->dev, "DMIC FIFO error %x\n", irq_status);
>> +
>> +       if (irq_status & OMAP_DMIC_IRQ_EMPTY)
>> +               dev_dbg(dmic->dev, "DMIC FIFO error %x\n", irq_status);
>> +
>> +       if (irq_status & OMAP_DMIC_IRQ)
>> +               dev_dbg(dmic->dev, "DMIC write request\n");
>
> no locking needed ??
>
>> +static int omap_dmic_dai_startup(struct snd_pcm_substream *substream,
>> +                                 struct snd_soc_dai *dai)
>> +{
>> +       struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
>> +
>> +       if (!dmic->active++)
>> +               pm_runtime_get_sync(dmic->dev);
>> +
>> +       return 0;
>> +}
>> +
>> +static void omap_dmic_dai_shutdown(struct snd_pcm_substream *substream,
>> +                                   struct snd_soc_dai *dai)
>> +{
>> +       struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
>> +
>> +       if (!--dmic->active)
>> +               pm_runtime_put_sync(dmic->dev);
>
> I could be wrong but I believe pm_runtime implementation on OMAP has
> its own refcounting, so you could drop the need for dmic->active.

This was a included for a future use case where the driver would be
accessed by the
Audio Back End.  In this case, there are multiple DAI's associated to
this driver and
we need to keep track of the number of active DAI's and only cal
pm_runtime_put_sync()
when there are no others running.

>
>> +static struct snd_soc_dai_ops omap_dmic_dai_ops = {
>
> should this be const ?
>
>> +static struct snd_soc_dai_driver omap_dmic_dai = {
>
> this too ??
>
>> +static __devinit int asoc_dmic_probe(struct platform_device *pdev)
>> +{
>> +       struct omap_dmic *dmic;
>> +       struct resource *res;
>> +       int ret;
>> +
>> +       dmic = kzalloc(sizeof(struct omap_dmic), GFP_KERNEL);
>> +       if (!dmic)
>> +               return -ENOMEM;
>> +
>> +       platform_set_drvdata(pdev, dmic);
>> +       dmic->dev = &pdev->dev;
>> +       dmic->link = &omap_dmic_link;
>> +       dmic->sysclk = OMAP_DMIC_SYSCLK_SYNC_MUX_CLKS;
>> +
>> +       spin_lock_init(&dmic->lock);
>> +
>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +       if (!res) {
>> +               dev_err(dmic->dev, "invalid memory resource\n");
>> +               ret = -ENODEV;
>> +               goto err_res;
>> +       }
>> +
>> +       dmic->io_base = ioremap(res->start, resource_size(res));
>> +       if (!dmic->io_base) {
>> +               ret = -ENOMEM;
>> +               goto err_res;
>> +       }
>> +
>> +       dmic->irq = platform_get_irq(pdev, 0);
>> +       if (dmic->irq < 0) {
>> +               ret = dmic->irq;
>> +               goto err_irq;
>> +       }
>> +
>> +       res = platform_get_resource(pdev, IORESOURCE_DMA, 0);
>> +       if (!res) {
>> +               dev_err(dmic->dev, "invalid dma resource\n");
>> +               ret = -ENODEV;
>> +               goto err_irq;
>> +       }
>> +       omap_dmic_dai_dma_params.dma_req = res->start;
>> +
>> +       pm_runtime_enable(dmic->dev);
>> +
>> +       /* Disable lines while request is ongoing */
>> +       omap_dmic_write(dmic, OMAP_DMIC_CTRL, 0x00);
>> +
>> +       ret = request_threaded_irq(dmic->irq, NULL, omap_dmic_irq_handler,
>> +                                  IRQF_ONESHOT, "DMIC", (void *)dmic);
>
> Does this need to be threaded ? Doesn't look like. Also, you don't need
> the (void *) cast.

This was originally not a threaded IRQ handler.  But in an internal
review, it was
suggested that this was the current trend in drivers and I should
follow that course.
I'm open to having it non-threaded.

>
>> +static int __devexit asoc_dmic_remove(struct platform_device *pdev)
>> +{
>> +       struct omap_dmic *dmic = platform_get_drvdata(pdev);
>> +
>> +       snd_soc_unregister_dai(&pdev->dev);
>> +       free_irq(dmic->irq, dmic);
>> +       iounmap(dmic->io_base);
>> +       kfree(dmic);
>
> pm_runtime_disable() ??
>
> --
> balbi
>

--
David Lambert
OMAP™ Multimedia
214-567-5692
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[Felipe Balbi]

Hi,

On Tue, Dec 28, 2010 at 07:13:58PM -0600, Lambert, David wrote:
>> one blank line only. BTW, are these used anywwhere outside the dmic.c
>> driver ? If not, it's better to move the definitions there.
>>
>
>They were originally in the omap-dmic.h header, but it was suggested
>that we move
>them to a platform header so that the driver could be more
>architecture independent
>and we could put the platform specific details here.  I'm OK putting
>them just about
>anywhere, as long as we have consensus.

The thing I don't like about putting register definitions under
<plat/*.h> is that it creates the need for making the driver "depends on
ARCH_OMAP" because it's the only architecture which has that file. If
you put under <linux/*> or directly on the .c source file, you can have
a much needed compile test with several architectures.

Even though the driver will never work with those other archs, compile
testing with several of them isn't bad at all.

>>> +#ifdef DEBUG
>>> +static void omap_dmic_reg_dump(struct omap_dmic *dmic)
>>> +{
>>> +       dev_dbg(dmic->dev, "***********************\n");
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_IRQSTATUS_RAW:  0x%04x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_IRQSTATUS_RAW));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_IRQSTATUS:  0x%04x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_IRQSTATUS));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_IRQENABLE_SET:  0x%04x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_IRQENABLE_SET));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_IRQENABLE_CLR:  0x%04x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_IRQENABLE_CLR));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_IRQWAKE_EN:  0x%04x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_IRQWAKE_EN));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_DMAENABLE_SET:  0x%04x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_DMAENABLE_SET));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_DMAENABLE_CLR:  0x%04x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_DMAENABLE_CLR));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_DMAWAKEEN:  0x%04x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_DMAWAKEEN));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_CTRL:  0x%04x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_CTRL));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_DATA:  0x%04x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_DATA));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_CTRL:  0x%04x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_FIFO_CTRL));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC1R_DATA:  0x%08x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC1R_DATA));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC1L_DATA:  0x%08x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC1L_DATA));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC2R_DATA:  0x%08x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC2R_DATA));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC2L_DATA:  0x%08x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC2L_DATA));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC3R_DATA:  0x%08x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC3R_DATA));
>>> +       dev_dbg(dmic->dev, "OMAP_DMIC_FIFO_DMIC3L_DATA:  0x%08x\n",
>>> +               omap_dmic_read(dmic, OMAP_DMIC_FIFO_DMIC3L_DATA));
>>> +       dev_dbg(dmic->dev, "***********************\n");
>>> +}
>>> +#else
>>> +static void omap_dmic_reg_dump(struct omap_dmic *dmic) {}
>>> +#endif
>>
>> Would be better to make a debugfs layer ??
>
>I'll look in to what it would take to do this.  Could this be a
>feature to add later?

It could, but this omap_dmic_reg_dump() really doesn't look nice. And
you even have a #undef DEBUG on the top of the file, which will require
code change to actually make this one work.

>>> +static irqreturn_t omap_dmic_irq_handler(int irq, void *dev_id)
>>> +{
>>> +       struct omap_dmic *dmic = dev_id;
>>> +       u32 irq_status;
>>> +
>>> +       irq_status = omap_dmic_read(dmic, OMAP_DMIC_IRQSTATUS);
>>> +
>>> +       /* Acknowledge irq event */
>>> +       omap_dmic_write(dmic, OMAP_DMIC_IRQSTATUS, irq_status);
>>> +       if (irq_status & OMAP_DMIC_IRQ_FULL)
>>> +               dev_dbg(dmic->dev, "DMIC FIFO error %x\n", irq_status);
>>> +
>>> +       if (irq_status & OMAP_DMIC_IRQ_EMPTY)
>>> +               dev_dbg(dmic->dev, "DMIC FIFO error %x\n", irq_status);
>>> +
>>> +       if (irq_status & OMAP_DMIC_IRQ)
>>> +               dev_dbg(dmic->dev, "DMIC write request\n");
>>
>> no locking needed ??
>>
>>> +static int omap_dmic_dai_startup(struct snd_pcm_substream *substream,
>>> +                                 struct snd_soc_dai *dai)
>>> +{
>>> +       struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
>>> +
>>> +       if (!dmic->active++)
>>> +               pm_runtime_get_sync(dmic->dev);
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static void omap_dmic_dai_shutdown(struct snd_pcm_substream *substream,
>>> +                                   struct snd_soc_dai *dai)
>>> +{
>>> +       struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
>>> +
>>> +       if (!--dmic->active)
>>> +               pm_runtime_put_sync(dmic->dev);
>>
>> I could be wrong but I believe pm_runtime implementation on OMAP has
>> its own refcounting, so you could drop the need for dmic->active.
>
>This was a included for a future use case where the driver would be
>accessed by the
>Audio Back End.  In this case, there are multiple DAI's associated to
>this driver and
>we need to keep track of the number of active DAI's and only cal
>pm_runtime_put_sync()
>when there are no others running.

Isn't it the same thing ?

DAI 0 -> omap_dmic_dai_startup() -> pm_runtime_get_sync() ->
dev->power.usage_count = 1

DAI 1 -> omap_dmic_dai_startup() -> pm_runtime_get_sync() ->
dev->power.usage_count = 2

DAI 2 -> omap_dmic_dai_startup() -> pm_runtime_get_sync() ->
dev->power.usage_count = 3

DAI 0 -> omap_dmic_dai_shutdown() -> pm_runtime_put_sync() ->
dev->power.usage_count = 2

and so on. Device will only be put in suspend again, when
dev->power.usage_count reaches zero.

>>> +       ret = request_threaded_irq(dmic->irq, NULL, omap_dmic_irq_handler,
>>> +                                  IRQF_ONESHOT, "DMIC", (void *)dmic);
>>
>> Does this need to be threaded ? Doesn't look like. Also, you don't need
>> the (void *) cast.
>
>This was originally not a threaded IRQ handler.  But in an internal
>review, it was
>suggested that this was the current trend in drivers and I should
>follow that course.
>I'm open to having it non-threaded.

Ok, no problem. But in this case you should have a non-threaded part
which will read the IRQ status register to verify we *do* have an IRQ
from this device and only in such case you will wake up the thread by
returning IRQ_WAKE_THREAD.

-- 
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[Liam Girdwood]

On Wed, 2010-12-29 at 11:47 +0200, Felipe Balbi wrote:
> Hi,
> 
> On Tue, Dec 28, 2010 at 07:13:58PM -0600, Lambert, David wrote:
> >> one blank line only. BTW, are these used anywwhere outside the dmic.c
> >> driver ? If not, it's better to move the definitions there.
> >>
> >
> >They were originally in the omap-dmic.h header, but it was suggested
> >that we move
> >them to a platform header so that the driver could be more
> >architecture independent
> >and we could put the platform specific details here.  I'm OK putting
> >them just about
> >anywhere, as long as we have consensus.
> 
> The thing I don't like about putting register definitions under
> <plat/*.h> is that it creates the need for making the driver "depends on
> ARCH_OMAP" because it's the only architecture which has that file. If
> you put under <linux/*> or directly on the .c source file, you can have
> a much needed compile test with several architectures.
> 
> Even though the driver will never work with those other archs, compile
> testing with several of them isn't bad at all.

This seems unnecessary since this driver is for the OMAP platform only
and also means maintainers will have to waste time fixing any build
issues for this driver on irrelevant architectures. The costs here
outweigh any benefits....

It also seems inconsistent with the other OMAP system headers in
plat-omap too.

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[Felipe Balbi]

Hi,

On Wed, Dec 29, 2010 at 10:35:31AM +0000, Liam Girdwood wrote:
>> Even though the driver will never work with those other archs, compile
>> testing with several of them isn't bad at all.
>
>This seems unnecessary since this driver is for the OMAP platform only
>and also means maintainers will have to waste time fixing any build
>issues for this driver on irrelevant architectures. The costs here
>outweigh any benefits....

If that's the case, what's the use for linux-next, then ? Drivers should
be arch independent as much as possible, no ? Isn't that why we don't
want drivers using branches to things such as machine_is_omap4_panda()
or similar ?

But the whole audio part on OMAP is still in a bad shape anyway, e.g.
mcbsp exports omap-only functions for drivers to use, so this will only
be yet another driver to the pile :-p

>It also seems inconsistent with the other OMAP system headers in
>plat-omap too.

Other than a few drivers which still need converting (and are on their
way) I can only see really arch-specific bits and pieces under plat/.

Not sure what's your point here.

-- 
balbi

Re: [PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[Liam Girdwood]

On Wed, 2010-12-29 at 12:44 +0200, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Dec 29, 2010 at 10:35:31AM +0000, Liam Girdwood wrote:
> >> Even though the driver will never work with those other archs, compile
> >> testing with several of them isn't bad at all.
> >
> >This seems unnecessary since this driver is for the OMAP platform only
> >and also means maintainers will have to waste time fixing any build
> >issues for this driver on irrelevant architectures. The costs here
> >outweigh any benefits....
> 
> If that's the case, what's the use for linux-next, then ? Drivers should
> be arch independent as much as possible, no ? Isn't that why we don't
> want drivers using branches to things such as machine_is_omap4_panda()
> or similar ?
> 

I agree that drivers should be arch independent when possible, but in
this case the OMAP DMIC DAI driver is coupled to the OMAP platform only.
i.e. it performs IO directly on the OMAP DMIC IP. This IP is only found
on OMAP silicon.

I also agree it would be nice if it builds for PXA, MIPS, SuperH etc but
what happens when the driver builds fine for OMAP but breaks the PXA,
MIPS, SuperH build ? Who will spend time to fix and test it ?

My main problem here is cost benefit. No one benefits directly by having
this driver available for the above platforms but it costs me time
fixing it when it breaks. 

> But the whole audio part on OMAP is still in a bad shape anyway, e.g.
> mcbsp exports omap-only functions for drivers to use, so this will only
> be yet another driver to the pile :-p
> 

In this case though the other McBSP user afaik is DaVinci, so in this
case it does make sense to make this driver support both.

> >It also seems inconsistent with the other OMAP system headers in
> >plat-omap too.
> 
> Other than a few drivers which still need converting (and are on their
> way) I can only see really arch-specific bits and pieces under plat/.
> 
> Not sure what's your point here.
> 

The point is that David had split the DMIC headers into two files, one
for plat specific registers and bit definitions and the other for audio
definitions (for the machine drivers) and is/was consistent with the
current OMAP platform headers. 

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[Mark Brown]

On Wed, Dec 29, 2010 at 11:52:51AM +0000, Liam Girdwood wrote:

> In this case though the other McBSP user afaik is DaVinci, so in this
> case it does make sense to make this driver support both.

The other thing with McBSP is that it's a generic programmable serial
port and so need not be tied to audio use (though as I understand it
other uses are very rare).

Re: [PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[Felipe Balbi]

Hi,

On Wed, Dec 29, 2010 at 11:56:28AM +0000, Mark Brown wrote:
>> In this case though the other McBSP user afaik is DaVinci, so in this
>> case it does make sense to make this driver support both.
>
>The other thing with McBSP is that it's a generic programmable serial
>port and so need not be tied to audio use (though as I understand it
>other uses are very rare).

So why is it still tied to OMAP-only ? There's no abstraction layer
around McBSP so OMAP McBSP driver has to export a whole bunch of symbols
for its users:

$ git grep -e EXPORT_SYMBOL arch/arm/*omap*/*mcbsp*.c
arch/arm/mach-omap2/mcbsp.c:EXPORT_SYMBOL(omap2_mcbsp1_mux_clkr_src);
arch/arm/mach-omap2/mcbsp.c:EXPORT_SYMBOL(omap2_mcbsp1_mux_fsr_src);
arch/arm/mach-omap2/mcbsp.c:EXPORT_SYMBOL(omap2_mcbsp_set_clks_src);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_config);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_st_set_chgain);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_st_get_chgain);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_st_enable);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_st_disable);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_st_is_enabled);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_set_tx_threshold);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_set_rx_threshold);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_get_max_tx_threshold);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_get_max_rx_threshold);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_get_fifo_size);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_get_tx_delay);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_get_rx_delay);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_get_dma_op_mode);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_set_io_type);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_request);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_free);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_start);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_stop);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_pollwrite);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_pollread);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_xmit_word);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_recv_word);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_spi_master_xmit_word_poll);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_spi_master_recv_word_poll);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_xmit_buffer);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_recv_buffer);
arch/arm/plat-omap/mcbsp.c:EXPORT_SYMBOL(omap_mcbsp_set_spi_mode);

-- 
balbi

Re: [PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[Mark Brown]

On Wed, Dec 29, 2010 at 01:59:24PM +0200, Felipe Balbi wrote:
> On Wed, Dec 29, 2010 at 11:56:28AM +0000, Mark Brown wrote:

> >The other thing with McBSP is that it's a generic programmable serial
> >port and so need not be tied to audio use (though as I understand it
> >other uses are very rare).

> So why is it still tied to OMAP-only ? There's no abstraction layer
> around McBSP so OMAP McBSP driver has to export a whole bunch of symbols
> for its users:

As far as I know the hardware has not been used on other platforms - all
the people who care specifically about the driver are using OMAP and
don't see any reason to do the work.  There's really no pressure to do
this.

Re: [PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[Felipe Balbi]

Hi,

On Wed, Dec 29, 2010 at 11:52:51AM +0000, Liam Girdwood wrote:
>I agree that drivers should be arch independent when possible, but in
>this case the OMAP DMIC DAI driver is coupled to the OMAP platform only.
>i.e. it performs IO directly on the OMAP DMIC IP. This IP is only found
>on OMAP silicon.

Let's imagine this IP is sourced from someone else and not created by
TI, what prevents company XYZ from sourcing the same IP and end up
re-using the driver ?

>I also agree it would be nice if it builds for PXA, MIPS, SuperH etc but
>what happens when the driver builds fine for OMAP but breaks the PXA,
>MIPS, SuperH build ? Who will spend time to fix and test it ?

You forgetting about other ARMs. This won't compile on DaVinci either.

>My main problem here is cost benefit. No one benefits directly by having
>this driver available for the above platforms but it costs me time
>fixing it when it breaks.

Of course there's people benefitting: OMAP audio driver writers would
know their driver is well written and doesn't break build for anybody.

Ok, let me put it this way:

What happens when you want have one kernel image for OMAP and DaVinci ?
Granted, that's not possible today anyway but e.g. Linaro is pushing for
it and IMO should be the way to go for us to be able to have a
distro-like kernel for ARM-based systems too.

>> >It also seems inconsistent with the other OMAP system headers in
>> >plat-omap too.
>>
>> Other than a few drivers which still need converting (and are on their
>> way) I can only see really arch-specific bits and pieces under plat/.
>>
>> Not sure what's your point here.
>>
>
>The point is that David had split the DMIC headers into two files, one
>for plat specific registers and bit definitions and the other for audio
>definitions (for the machine drivers) and is/was consistent with the
>current OMAP platform headers.

You forgot to mention that plat/ headers are supposed to be used by
plat-* and mach-* code only to setup the correct state for the driver.
Just recently one of the musb glue layers got broken because it was
mistakenly using <plat/control.h> and that got moved.

That's actually my point and why I think drivers should not touch plat/*
and mach/* headers.

-- 
balbi

Re: [PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[Liam Girdwood]

On Wed, 2010-12-29 at 14:04 +0200, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Dec 29, 2010 at 11:52:51AM +0000, Liam Girdwood wrote:
> >I agree that drivers should be arch independent when possible, but in
> >this case the OMAP DMIC DAI driver is coupled to the OMAP platform only.
> >i.e. it performs IO directly on the OMAP DMIC IP. This IP is only found
> >on OMAP silicon.
> 
> Let's imagine this IP is sourced from someone else and not created by
> TI, what prevents company XYZ from sourcing the same IP and end up
> re-using the driver ?

That's fine, but this is not the case atm for this particular DMIC IP
and we can only justify effort for OMAP atm.

> 
> >I also agree it would be nice if it builds for PXA, MIPS, SuperH etc but
> >what happens when the driver builds fine for OMAP but breaks the PXA,
> >MIPS, SuperH build ? Who will spend time to fix and test it ?
> 
> You forgetting about other ARMs. This won't compile on DaVinci either.
> 
> >My main problem here is cost benefit. No one benefits directly by having
> >this driver available for the above platforms but it costs me time
> >fixing it when it breaks.
> 
> Of course there's people benefitting: OMAP audio driver writers would
> know their driver is well written and doesn't break build for anybody.
> 
> Ok, let me put it this way:
> 
> What happens when you want have one kernel image for OMAP and DaVinci ?
> Granted, that's not possible today anyway but e.g. Linaro is pushing for
> it and IMO should be the way to go for us to be able to have a
> distro-like kernel for ARM-based systems too.
> 

Ok, I now think you meant other "ARM architectures" here than other
Linux architectures in general. It does make a more sense for ARM
distribution deployment, but I still think guaranteeing successful build
for this driver on all non ARM architectures atm just to give us
satisfaction that the driver is "well written" is unnecessary effort.

> >> >It also seems inconsistent with the other OMAP system headers in
> >> >plat-omap too.
> >>
> >> Other than a few drivers which still need converting (and are on their
> >> way) I can only see really arch-specific bits and pieces under plat/.
> >>
> >> Not sure what's your point here.
> >>
> >
> >The point is that David had split the DMIC headers into two files, one
> >for plat specific registers and bit definitions and the other for audio
> >definitions (for the machine drivers) and is/was consistent with the
> >current OMAP platform headers.
> 
> You forgot to mention that plat/ headers are supposed to be used by
> plat-* and mach-* code only to setup the correct state for the driver.
> Just recently one of the musb glue layers got broken because it was
> mistakenly using <plat/control.h> and that got moved.
> 
> That's actually my point and why I think drivers should not touch plat/*
> and mach/* headers.
> 

In a lot of cases drivers under drivers/* and sound/* must touch plat/*
and mach/* headers for runtime access to hardware IP. I've just grepped
and the list is quite long for drivers/*

Liam

Re: [PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[Felipe Balbi]

Hi,

On Wed, Dec 29, 2010 at 01:00:00PM +0000, Liam Girdwood wrote:
>Ok, I now think you meant other "ARM architectures" here than other
>Linux architectures in general. It does make a more sense for ARM
>distribution deployment, but I still think guaranteeing successful build
>for this driver on all non ARM architectures atm just to give us
>satisfaction that the driver is "well written" is unnecessary effort.

Ok.

>In a lot of cases drivers under drivers/* and sound/* must touch plat/*
>and mach/* headers for runtime access to hardware IP. I've just grepped
>and the list is quite long for drivers/*

IMHO that's wrong :-p But that's only my opinion. Anyway, I'm ok with
your decision FWIW.

-- 
balbi

Re: [PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[Mark Brown]

On Mon, Dec 27, 2010 at 10:17:02PM -0600, David Lambert wrote:

> +	case 19200000:
> +		if (div == 5)
> +			div_sel = 0x1;
> +		else if (div == 8)
> +			div_sel = 0x0;
> +		break;

A switch statement for the valid div values would feel more natural,
together with a default case to report errors though that's handled
below so could be skipped.

> +	/* Acknowledge irq event */
> +	omap_dmic_write(dmic, OMAP_DMIC_IRQSTATUS, irq_status);
> +	if (irq_status & OMAP_DMIC_IRQ_FULL)

Blank line between these two.

> +		dev_dbg(dmic->dev, "DMIC FIFO error %x\n", irq_status);
> +
> +	if (irq_status & OMAP_DMIC_IRQ_EMPTY)
> +		dev_dbg(dmic->dev, "DMIC FIFO error %x\n", irq_status);

I'd expect these errors to be displayed by default.

> +	if (irq_status & OMAP_DMIC_IRQ)
> +		dev_dbg(dmic->dev, "DMIC write request\n");

A comment explaining why we don't actually do anything with the request
would be helpful.  Given that we're ignoring the event it'd seem better
to just leave it masked and not take the interrupt in the first place.

> +static int omap_dmic_dai_startup(struct snd_pcm_substream *substream,
> +				  struct snd_soc_dai *dai)
> +{
> +	struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
> +
> +	if (!dmic->active++)
> +		pm_runtime_get_sync(dmic->dev);

The pm_runtime API does refcounting for you so you should be able to
just skip the active count check here, IIRC there's a pm_runtime API you
could use to query if the DMIC is enabled so you could skip the local
refcount entirely.

> +static int omap_dmic_dai_hw_free(struct snd_pcm_substream *substream,
> +				  struct snd_soc_dai *dai)
> +{
> +	struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
> +	struct omap_dmic_link *link = dmic->link;
> +	int ret = 0;
> +
> +	if (dmic->active == 1) {
> +		ret = omap_dmic_close(dmic);
> +		link->channels = 0;
> +	}

I'd feel a bit more happy if this were done in the same place as the
drop of the refcount though this should actually be OK I think.

> +	switch (freq) {
> +	case 19200000:
> +	case 24000000:
> +	case 24576000:
> +	case 12000000:
> +		dmic->clk_freq = freq;
> +		break;
> +	default:
> +		dev_err(dai->dev, "clk freq not supported %d\n", freq);
> +		ret = -EINVAL;
> +		goto err_freq;
> +	}

Can't you ask the parent clock what rate it's set to?

> +	ret = request_threaded_irq(dmic->irq, NULL, omap_dmic_irq_handler,
> +				   IRQF_ONESHOT, "DMIC", (void *)dmic);
> +	if (ret) {
> +		dev_err(dmic->dev, "irq request failed\n");
> +		goto err_irq;
> +	}

Does this really need to be a threaded IRQ - the IRQ was just reading
from CPU registers as far as I remember?

> +MODULE_AUTHOR("David Lambert <dlambert@ti.com>");
> +MODULE_DESCRIPTION("OMAP DMIC SoC Interface");
> +MODULE_LICENSE("GPL");

Should have a MODULE_ALIAS too in case it gets built modular.

Re: [PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[Lambert, David]

Mark,

On Tue, Dec 28, 2010 at 8:18 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Dec 27, 2010 at 10:17:02PM -0600, David Lambert wrote:
>
>> +     case 19200000:
>> +             if (div == 5)
>> +                     div_sel = 0x1;
>> +             else if (div == 8)
>> +                     div_sel = 0x0;
>> +             break;
>
> A switch statement for the valid div values would feel more natural,
> together with a default case to report errors though that's handled
> below so could be skipped.
>
>> +     /* Acknowledge irq event */
>> +     omap_dmic_write(dmic, OMAP_DMIC_IRQSTATUS, irq_status);
>> +     if (irq_status & OMAP_DMIC_IRQ_FULL)
>
> Blank line between these two.
>
>> +             dev_dbg(dmic->dev, "DMIC FIFO error %x\n", irq_status);
>> +
>> +     if (irq_status & OMAP_DMIC_IRQ_EMPTY)
>> +             dev_dbg(dmic->dev, "DMIC FIFO error %x\n", irq_status);
>
> I'd expect these errors to be displayed by default.
>
>> +     if (irq_status & OMAP_DMIC_IRQ)
>> +             dev_dbg(dmic->dev, "DMIC write request\n");
>
> A comment explaining why we don't actually do anything with the request
> would be helpful.  Given that we're ignoring the event it'd seem better
> to just leave it masked and not take the interrupt in the first place.

Since the IRQ handler isn't really doing anything in this driver,
would it better
to just not have one?

>
>> +static int omap_dmic_dai_startup(struct snd_pcm_substream *substream,
>> +                               struct snd_soc_dai *dai)
>> +{
>> +     struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
>> +
>> +     if (!dmic->active++)
>> +             pm_runtime_get_sync(dmic->dev);
>
> The pm_runtime API does refcounting for you so you should be able to
> just skip the active count check here, IIRC there's a pm_runtime API you
> could use to query if the DMIC is enabled so you could skip the local
> refcount entirely.
>

I haven't been able to find a an API to query it, but I could just
reference the count
directly with dmic->dev->power.usage_count.

>> +static int omap_dmic_dai_hw_free(struct snd_pcm_substream *substream,
>> +                               struct snd_soc_dai *dai)
>> +{
>> +     struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
>> +     struct omap_dmic_link *link = dmic->link;
>> +     int ret = 0;
>> +
>> +     if (dmic->active == 1) {
>> +             ret = omap_dmic_close(dmic);
>> +             link->channels = 0;
>> +     }
>
> I'd feel a bit more happy if this were done in the same place as the
> drop of the refcount though this should actually be OK I think.
>
>> +     switch (freq) {
>> +     case 19200000:
>> +     case 24000000:
>> +     case 24576000:
>> +     case 12000000:
>> +             dmic->clk_freq = freq;
>> +             break;
>> +     default:
>> +             dev_err(dai->dev, "clk freq not supported %d\n", freq);
>> +             ret = -EINVAL;
>> +             goto err_freq;
>> +     }
>
> Can't you ask the parent clock what rate it's set to?
>

Didn't really think there was a need for that on the audio driver.
There are clock API's available
to query that already.

>> +     ret = request_threaded_irq(dmic->irq, NULL, omap_dmic_irq_handler,
>> +                                IRQF_ONESHOT, "DMIC", (void *)dmic);
>> +     if (ret) {
>> +             dev_err(dmic->dev, "irq request failed\n");
>> +             goto err_irq;
>> +     }
>
> Does this really need to be a threaded IRQ - the IRQ was just reading
> from CPU registers as far as I remember?
>
>> +MODULE_AUTHOR("David Lambert <dlambert@ti.com>");
>> +MODULE_DESCRIPTION("OMAP DMIC SoC Interface");
>> +MODULE_LICENSE("GPL");
>
> Should have a MODULE_ALIAS too in case it gets built modular.
>

--
David Lambert
OMAP™ Multimedia
214-567-5692
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] ASoC: DMIC: Adding the OMAP DMIC driver

[Mark Brown]

On Wed, Jan 05, 2011 at 07:56:13AM -0600, Lambert, David wrote:
> On Tue, Dec 28, 2010 at 8:18 AM, Mark Brown

> > A comment explaining why we don't actually do anything with the request
> > would be helpful.  Given that we're ignoring the event it'd seem better
> > to just leave it masked and not take the interrupt in the first place.

> Since the IRQ handler isn't really doing anything in this driver,
> would it better
> to just not have one?

Possibly, if the hardware doesn't care.

> >> +             dmic->clk_freq = freq;
> >> +             break;
> >> +     default:
> >> +             dev_err(dai->dev, "clk freq not supported %d\n", freq);
> >> +             ret = -EINVAL;
> >> +             goto err_freq;
> >> +     }

> > Can't you ask the parent clock what rate it's set to?

> Didn't really think there was a need for that on the audio driver.
> There are clock API's available
> to query that already.

That's my point - you're requiring that the user pass in the rate rather
than querying the clock API.

[PATCH 2/5] ASoC: DMIC codec: Adding a generic DMIC codec

[David Lambert]

This codec is to be used by the DMIC driver to
control the DMIC codec.  This driver will be used on future implementations
of the DMIC driver to support codec specific features.

At this time, the codec driver just registers the codec DAI.

Signed-off-by: David Lambert <dlambert@ti.com>
---
 sound/soc/codecs/dmic.c |   79 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 79 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/codecs/dmic.c

diff --git a/sound/soc/codecs/dmic.c b/sound/soc/codecs/dmic.c
new file mode 100644
index 0000000..6f54bc5
--- /dev/null
+++ b/sound/soc/codecs/dmic.c
@@ -0,0 +1,79 @@
+/*
+ * dmic.c  --  SoC audio for Generic Digital MICs
+ *
+ * Author: Liam Girdwood <lrg@slimlogic.co.uk>
+ *
+ * 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/platform_device.h>
+#include <linux/slab.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <sound/soc-dapm.h>
+
+static struct snd_soc_dai_driver dmic_dai = {
+	.name = "dmic-hifi",
+	.capture = {
+		.stream_name = "Capture",
+		.channels_min = 1,
+		.channels_max = 8,
+		.rates = SNDRV_PCM_RATE_CONTINUOUS,
+		.formats = SNDRV_PCM_FMTBIT_S32_LE
+			| SNDRV_PCM_FMTBIT_S24_LE
+			| SNDRV_PCM_FMTBIT_S16_LE,
+	},
+};
+
+static struct snd_soc_codec_driver soc_dmic = {};
+
+static int __devinit dmic_dev_probe(struct platform_device *pdev)
+{
+	return snd_soc_register_codec(&pdev->dev,
+			&soc_dmic, &dmic_dai, 1);
+}
+
+static int __devexit dmic_dev_remove(struct platform_device *pdev)
+{
+	snd_soc_unregister_codec(&pdev->dev);
+	return 0;
+}
+
+static struct platform_driver dmic_driver = {
+	.driver = {
+		.name = "dmic-codec",
+		.owner = THIS_MODULE,
+	},
+	.probe = dmic_dev_probe,
+	.remove = __devexit_p(dmic_dev_remove),
+};
+
+static int __init dmic_init(void)
+{
+	return platform_driver_register(&dmic_driver);
+}
+module_init(dmic_init);
+
+static void __exit dmic_exit(void)
+{
+	platform_driver_unregister(&dmic_driver);
+}
+module_exit(dmic_exit);
+
+MODULE_DESCRIPTION("Generic DMIC driver");
+MODULE_AUTHOR("Liam Girdwood <lrg@slimlogic.co.uk>");
+MODULE_LICENSE("GPL");
-- 
1.7.0.4

Re: [PATCH 2/5] ASoC: DMIC codec: Adding a generic DMIC codec

[Mark Brown]

On Mon, Dec 27, 2010 at 10:17:03PM -0600, David Lambert wrote:

> At this time, the codec driver just registers the codec DAI.

If it's doing more than that it's going to be a separate driver anyway I
expect.

> +static struct snd_soc_dai_driver dmic_dai = {
> +	.name = "dmic-hifi",
> +	.capture = {
> +		.stream_name = "Capture",
> +		.channels_min = 1,
> +		.channels_max = 8,
> +		.rates = SNDRV_PCM_RATE_CONTINUOUS,
> +		.formats = SNDRV_PCM_FMTBIT_S32_LE
> +			| SNDRV_PCM_FMTBIT_S24_LE
> +			| SNDRV_PCM_FMTBIT_S16_LE,

Hrm, so.  DMIC signals are inherantly stereo, though obviously you can
bind multiple DMICs in parallel using the same clock line so from the
CPU side the channel limit makes sense.  Similarly the format here
really makes very little odds on a PDM interface.

What we probably want here is wildcards for both channel and format
limits which we can use to say "this is meaningless, just match
anything" in case the other end needs it (like a CPU PDM interface does
to set up the DMA).

> +static struct platform_driver dmic_driver = {
> +	.driver = {
> +		.name = "dmic-codec",

Just -dmic.

> +MODULE_DESCRIPTION("Generic DMIC driver");
> +MODULE_AUTHOR("Liam Girdwood <lrg@slimlogic.co.uk>");
> +MODULE_LICENSE("GPL");

MODULE_ALIAS().

[PATCH 3/5] ASoC: DMIC: Adding OMAP DMIC driver to build

[David Lambert]

Adds the OMAP DMIC driver and codec to the build.

Signed-off-by: David Lambert <dlambert@ti.com>
---
 sound/soc/codecs/Kconfig  |    3 +++
 sound/soc/codecs/Makefile |    2 ++
 sound/soc/omap/Kconfig    |    5 +++++
 sound/soc/omap/Makefile   |    2 ++
 4 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 0f33db2..f6c6d31 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -166,6 +166,9 @@ config SND_SOC_L3
 config SND_SOC_DA7210
         tristate
 
+config SND_SOC_DMIC
+	tristate
+
 config SND_SOC_MAX98088
        tristate
 
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 10e5e09..9139cf9 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -14,6 +14,7 @@ snd-soc-cs42l51-objs := cs42l51.o
 snd-soc-cs4270-objs := cs4270.o
 snd-soc-cx20442-objs := cx20442.o
 snd-soc-da7210-objs := da7210.o
+snd-soc-dmic-objs := dmic.o
 snd-soc-l3-objs := l3.o
 snd-soc-max98088-objs := max98088.o
 snd-soc-pcm3008-objs := pcm3008.o
@@ -91,6 +92,7 @@ obj-$(CONFIG_SND_SOC_CS42L51)	+= snd-soc-cs42l51.o
 obj-$(CONFIG_SND_SOC_CS4270)	+= snd-soc-cs4270.o
 obj-$(CONFIG_SND_SOC_CX20442)	+= snd-soc-cx20442.o
 obj-$(CONFIG_SND_SOC_DA7210)	+= snd-soc-da7210.o
+obj-$(CONFIG_SND_SOC_DMIC)	+= snd-soc-dmic.o
 obj-$(CONFIG_SND_SOC_L3)	+= snd-soc-l3.o
 obj-$(CONFIG_SND_SOC_JZ4740_CODEC)	+= snd-soc-jz4740-codec.o
 obj-$(CONFIG_SND_SOC_MAX98088)	+= snd-soc-max98088.o
diff --git a/sound/soc/omap/Kconfig b/sound/soc/omap/Kconfig
index a088db6..548b13c 100644
--- a/sound/soc/omap/Kconfig
+++ b/sound/soc/omap/Kconfig
@@ -9,6 +9,9 @@ config SND_OMAP_SOC_MCBSP
 config SND_OMAP_SOC_MCPDM
 	tristate
 
+config SND_OMAP_SOC_DMIC
+	tristate
+
 config SND_OMAP_SOC_N810
 	tristate "SoC Audio support for Nokia N810"
 	depends on SND_OMAP_SOC && MACH_NOKIA_N810 && I2C
@@ -103,6 +106,8 @@ config SND_OMAP_SOC_SDP4430
 	depends on TWL4030_CORE && SND_OMAP_SOC && MACH_OMAP_4430SDP
 	select SND_OMAP_SOC_MCPDM
 	select SND_SOC_TWL6040
+	select SND_SOC_DMIC
+	select SND_OMAP_SOC_DMIC
 	help
 	  Say Y if you want to add support for SoC audio on Texas Instruments
 	  SDP4430.
diff --git a/sound/soc/omap/Makefile b/sound/soc/omap/Makefile
index ba9fc65..6ff27f5 100644
--- a/sound/soc/omap/Makefile
+++ b/sound/soc/omap/Makefile
@@ -1,9 +1,11 @@
 # OMAP Platform Support
 snd-soc-omap-objs := omap-pcm.o
+snd-soc-omap-dmic-objs := omap-dmic.o
 snd-soc-omap-mcbsp-objs := omap-mcbsp.o
 snd-soc-omap-mcpdm-objs := omap-mcpdm.o mcpdm.o
 
 obj-$(CONFIG_SND_OMAP_SOC) += snd-soc-omap.o
+obj-$(CONFIG_SND_OMAP_SOC_DMIC) += snd-soc-omap-dmic.o
 obj-$(CONFIG_SND_OMAP_SOC_MCBSP) += snd-soc-omap-mcbsp.o
 obj-$(CONFIG_SND_OMAP_SOC_MCPDM) += snd-soc-omap-mcpdm.o
 
-- 
1.7.0.4

Re: [PATCH 3/5] ASoC: DMIC: Adding OMAP DMIC driver to build

[Mark Brown]

On Mon, Dec 27, 2010 at 10:17:04PM -0600, David Lambert wrote:
> Adds the OMAP DMIC driver and codec to the build.
> 
> Signed-off-by: David Lambert <dlambert@ti.com>
> ---
>  sound/soc/codecs/Kconfig  |    3 +++
>  sound/soc/codecs/Makefile |    2 ++
>  sound/soc/omap/Kconfig    |    5 +++++
>  sound/soc/omap/Makefile   |    2 ++

This should be one of the last patches in the series, but given the sort
of drivers you're introducing I'd expect the drivers to just directly
add the build system stuff in the patch.

>  4 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index 0f33db2..f6c6d31 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -166,6 +166,9 @@ config SND_SOC_L3
>  config SND_SOC_DA7210
>          tristate
>  
> +config SND_SOC_DMIC
> +	tristate
> +
>  config SND_SOC_MAX98088
>         tristate
>  
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index 10e5e09..9139cf9 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -14,6 +14,7 @@ snd-soc-cs42l51-objs := cs42l51.o
>  snd-soc-cs4270-objs := cs4270.o
>  snd-soc-cx20442-objs := cx20442.o
>  snd-soc-da7210-objs := da7210.o
> +snd-soc-dmic-objs := dmic.o
>  snd-soc-l3-objs := l3.o
>  snd-soc-max98088-objs := max98088.o
>  snd-soc-pcm3008-objs := pcm3008.o
> @@ -91,6 +92,7 @@ obj-$(CONFIG_SND_SOC_CS42L51)	+= snd-soc-cs42l51.o
>  obj-$(CONFIG_SND_SOC_CS4270)	+= snd-soc-cs4270.o
>  obj-$(CONFIG_SND_SOC_CX20442)	+= snd-soc-cx20442.o
>  obj-$(CONFIG_SND_SOC_DA7210)	+= snd-soc-da7210.o
> +obj-$(CONFIG_SND_SOC_DMIC)	+= snd-soc-dmic.o
>  obj-$(CONFIG_SND_SOC_L3)	+= snd-soc-l3.o
>  obj-$(CONFIG_SND_SOC_JZ4740_CODEC)	+= snd-soc-jz4740-codec.o
>  obj-$(CONFIG_SND_SOC_MAX98088)	+= snd-soc-max98088.o
> diff --git a/sound/soc/omap/Kconfig b/sound/soc/omap/Kconfig
> index a088db6..548b13c 100644
> --- a/sound/soc/omap/Kconfig
> +++ b/sound/soc/omap/Kconfig
> @@ -9,6 +9,9 @@ config SND_OMAP_SOC_MCBSP
>  config SND_OMAP_SOC_MCPDM
>  	tristate
>  
> +config SND_OMAP_SOC_DMIC
> +	tristate
> +
>  config SND_OMAP_SOC_N810
>  	tristate "SoC Audio support for Nokia N810"
>  	depends on SND_OMAP_SOC && MACH_NOKIA_N810 && I2C
> @@ -103,6 +106,8 @@ config SND_OMAP_SOC_SDP4430
>  	depends on TWL4030_CORE && SND_OMAP_SOC && MACH_OMAP_4430SDP
>  	select SND_OMAP_SOC_MCPDM
>  	select SND_SOC_TWL6040
> +	select SND_SOC_DMIC
> +	select SND_OMAP_SOC_DMIC
>  	help
>  	  Say Y if you want to add support for SoC audio on Texas Instruments
>  	  SDP4430.
> diff --git a/sound/soc/omap/Makefile b/sound/soc/omap/Makefile
> index ba9fc65..6ff27f5 100644
> --- a/sound/soc/omap/Makefile
> +++ b/sound/soc/omap/Makefile
> @@ -1,9 +1,11 @@
>  # OMAP Platform Support
>  snd-soc-omap-objs := omap-pcm.o
> +snd-soc-omap-dmic-objs := omap-dmic.o
>  snd-soc-omap-mcbsp-objs := omap-mcbsp.o
>  snd-soc-omap-mcpdm-objs := omap-mcpdm.o mcpdm.o
>  
>  obj-$(CONFIG_SND_OMAP_SOC) += snd-soc-omap.o
> +obj-$(CONFIG_SND_OMAP_SOC_DMIC) += snd-soc-omap-dmic.o
>  obj-$(CONFIG_SND_OMAP_SOC_MCBSP) += snd-soc-omap-mcbsp.o
>  obj-$(CONFIG_SND_OMAP_SOC_MCPDM) += snd-soc-omap-mcpdm.o
>  
> -- 
> 1.7.0.4
> 

Re: [PATCH 3/5] ASoC: DMIC: Adding OMAP DMIC driver to build

[Lambert, David]

On Tue, Dec 28, 2010 at 5:40 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Mon, Dec 27, 2010 at 10:17:04PM -0600, David Lambert wrote:
>> Adds the OMAP DMIC driver and codec to the build.
>>
>> Signed-off-by: David Lambert <dlambert@ti.com>
>> ---
>>  sound/soc/codecs/Kconfig  |    3 +++
>>  sound/soc/codecs/Makefile |    2 ++
>>  sound/soc/omap/Kconfig    |    5 +++++
>>  sound/soc/omap/Makefile   |    2 ++
>
> This should be one of the last patches in the series, but given the sort
> of drivers you're introducing I'd expect the drivers to just directly
> add the build system stuff in the patch.
>

OK... I can put the makefile and kconfig changes in with the DMIC driver and the
DMIC codec.

>>  4 files changed, 12 insertions(+), 0 deletions(-)
>>
>> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
>> index 0f33db2..f6c6d31 100644
>> --- a/sound/soc/codecs/Kconfig
>> +++ b/sound/soc/codecs/Kconfig
>> @@ -166,6 +166,9 @@ config SND_SOC_L3
>>  config SND_SOC_DA7210
>>          tristate
>>
>> +config SND_SOC_DMIC
>> +     tristate
>> +
>>  config SND_SOC_MAX98088
>>         tristate
>>
>> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
>> index 10e5e09..9139cf9 100644
>> --- a/sound/soc/codecs/Makefile
>> +++ b/sound/soc/codecs/Makefile
>> @@ -14,6 +14,7 @@ snd-soc-cs42l51-objs := cs42l51.o
>>  snd-soc-cs4270-objs := cs4270.o
>>  snd-soc-cx20442-objs := cx20442.o
>>  snd-soc-da7210-objs := da7210.o
>> +snd-soc-dmic-objs := dmic.o
>>  snd-soc-l3-objs := l3.o
>>  snd-soc-max98088-objs := max98088.o
>>  snd-soc-pcm3008-objs := pcm3008.o
>> @@ -91,6 +92,7 @@ obj-$(CONFIG_SND_SOC_CS42L51)       += snd-soc-cs42l51.o
>>  obj-$(CONFIG_SND_SOC_CS4270) += snd-soc-cs4270.o
>>  obj-$(CONFIG_SND_SOC_CX20442)        += snd-soc-cx20442.o
>>  obj-$(CONFIG_SND_SOC_DA7210) += snd-soc-da7210.o
>> +obj-$(CONFIG_SND_SOC_DMIC)   += snd-soc-dmic.o
>>  obj-$(CONFIG_SND_SOC_L3)     += snd-soc-l3.o
>>  obj-$(CONFIG_SND_SOC_JZ4740_CODEC)   += snd-soc-jz4740-codec.o
>>  obj-$(CONFIG_SND_SOC_MAX98088)       += snd-soc-max98088.o
>> diff --git a/sound/soc/omap/Kconfig b/sound/soc/omap/Kconfig
>> index a088db6..548b13c 100644
>> --- a/sound/soc/omap/Kconfig
>> +++ b/sound/soc/omap/Kconfig
>> @@ -9,6 +9,9 @@ config SND_OMAP_SOC_MCBSP
>>  config SND_OMAP_SOC_MCPDM
>>       tristate
>>
>> +config SND_OMAP_SOC_DMIC
>> +     tristate
>> +
>>  config SND_OMAP_SOC_N810
>>       tristate "SoC Audio support for Nokia N810"
>>       depends on SND_OMAP_SOC && MACH_NOKIA_N810 && I2C
>> @@ -103,6 +106,8 @@ config SND_OMAP_SOC_SDP4430
>>       depends on TWL4030_CORE && SND_OMAP_SOC && MACH_OMAP_4430SDP
>>       select SND_OMAP_SOC_MCPDM
>>       select SND_SOC_TWL6040
>> +     select SND_SOC_DMIC
>> +     select SND_OMAP_SOC_DMIC
>>       help
>>         Say Y if you want to add support for SoC audio on Texas Instruments
>>         SDP4430.
>> diff --git a/sound/soc/omap/Makefile b/sound/soc/omap/Makefile
>> index ba9fc65..6ff27f5 100644
>> --- a/sound/soc/omap/Makefile
>> +++ b/sound/soc/omap/Makefile
>> @@ -1,9 +1,11 @@
>>  # OMAP Platform Support
>>  snd-soc-omap-objs := omap-pcm.o
>> +snd-soc-omap-dmic-objs := omap-dmic.o
>>  snd-soc-omap-mcbsp-objs := omap-mcbsp.o
>>  snd-soc-omap-mcpdm-objs := omap-mcpdm.o mcpdm.o
>>
>>  obj-$(CONFIG_SND_OMAP_SOC) += snd-soc-omap.o
>> +obj-$(CONFIG_SND_OMAP_SOC_DMIC) += snd-soc-omap-dmic.o
>>  obj-$(CONFIG_SND_OMAP_SOC_MCBSP) += snd-soc-omap-mcbsp.o
>>  obj-$(CONFIG_SND_OMAP_SOC_MCPDM) += snd-soc-omap-mcpdm.o
>>
>> --
>> 1.7.0.4
>>
>


--
David Lambert
OMAP™ Multimedia
214-567-5692
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/5] OMAP4: hwmod: add entries for DMIC driver

[David Lambert]

Adds HWMOD entries for the OMAP DMIC driver and creates
a platform device.  The HWMOD entires define the system 
resource requirements for the drvier such as DMA addresses, channels,
and IRQ's.  Placing this information in the HWMOD database allows
for more generic drivers to be written and having the specific
implementation details defined in HWMOD.

Signed-off-by: David Lambert <dlambert@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |   91 ++++++++++++++++++++++++++++
 arch/arm/plat-omap/devices.c               |   35 +++++++++++
 2 files changed, 126 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 7274db4..f9b2ad3 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -383,6 +383,95 @@ static struct omap_hwmod omap44xx_l4_wkup_hwmod = {
 };
 
 /*
+ * 'dmic' class
+ * digital microphone controller
+ */
+
+static struct omap_hwmod_class_sysconfig omap44xx_dmic_sysc = {
+	.rev_offs	= 0x0000,
+	.sysc_offs	= 0x0010,
+	.sysc_flags	= (SYSC_HAS_EMUFREE | SYSC_HAS_RESET_STATUS |
+			   SYSC_HAS_SIDLEMODE | SYSC_HAS_SOFTRESET),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type2,
+};
+
+static struct omap_hwmod_class omap44xx_dmic_hwmod_class = {
+	.name = "omap-dmic",
+	.sysc = &omap44xx_dmic_sysc,
+};
+
+/* dmic */
+static struct omap_hwmod omap44xx_dmic_hwmod;
+static struct omap_hwmod_irq_info omap44xx_dmic_irqs[] = {
+	{ .irq = 114 + OMAP44XX_IRQ_GIC_START },
+};
+
+static struct omap_hwmod_dma_info omap44xx_dmic_sdma_reqs[] = {
+	{ .dma_req = 66 + OMAP44XX_DMA_REQ_START },
+};
+
+static struct omap_hwmod_addr_space omap44xx_dmic_addrs[] = {
+	{
+		.pa_start	= 0x4012e000,
+		.pa_end		= 0x4012e07f,
+		.flags		= ADDR_TYPE_RT
+	},
+};
+
+/* l4_abe -> dmic */
+static struct omap_hwmod_ocp_if omap44xx_l4_abe__dmic = {
+	.master		= &omap44xx_l4_abe_hwmod,
+	.slave		= &omap44xx_dmic_hwmod,
+	.clk		= "ocp_abe_iclk",
+	.addr		= omap44xx_dmic_addrs,
+	.addr_cnt	= ARRAY_SIZE(omap44xx_dmic_addrs),
+	.user		= OCP_USER_MPU,
+};
+
+static struct omap_hwmod_addr_space omap44xx_dmic_dma_addrs[] = {
+	{
+		.pa_start	= 0x4902e000,
+		.pa_end		= 0x4902e07f,
+		.flags		= ADDR_TYPE_RT
+	},
+};
+
+/* l4_abe -> dmic (dma) */
+static struct omap_hwmod_ocp_if omap44xx_l4_abe__dmic_dma = {
+	.master		= &omap44xx_l4_abe_hwmod,
+	.slave		= &omap44xx_dmic_hwmod,
+	.clk		= "ocp_abe_iclk",
+	.addr		= omap44xx_dmic_dma_addrs,
+	.addr_cnt	= ARRAY_SIZE(omap44xx_dmic_dma_addrs),
+	.user		= OCP_USER_SDMA,
+};
+
+/* dmic slave ports */
+static struct omap_hwmod_ocp_if *omap44xx_dmic_slaves[] = {
+	&omap44xx_l4_abe__dmic,
+	&omap44xx_l4_abe__dmic_dma,
+};
+
+static struct omap_hwmod omap44xx_dmic_hwmod = {
+	.name		= "omap-dmic",
+	.class		= &omap44xx_dmic_hwmod_class,
+	.mpu_irqs	= omap44xx_dmic_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap44xx_dmic_irqs),
+	.sdma_reqs	= omap44xx_dmic_sdma_reqs,
+	.sdma_reqs_cnt	= ARRAY_SIZE(omap44xx_dmic_sdma_reqs),
+	.main_clk	= "dmic_fck",
+	.prcm = {
+		.omap4 = {
+			.clkctrl_reg = OMAP4430_CM1_ABE_DMIC_CLKCTRL,
+		},
+	},
+	.slaves		= omap44xx_dmic_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap44xx_dmic_slaves),
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
+};
+
+/*
  * 'mpu_bus' class
  * instance(s): mpu_private
  */
@@ -826,6 +915,8 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
 	&omap44xx_l4_cfg_hwmod,
 	&omap44xx_l4_per_hwmod,
 	&omap44xx_l4_wkup_hwmod,
+	/* dmic class */
+	&omap44xx_dmic_hwmod,
 	/* mpu_bus class */
 	&omap44xx_mpu_private_hwmod,
 
diff --git a/arch/arm/plat-omap/devices.c b/arch/arm/plat-omap/devices.c
index fc81912..b347f75 100644
--- a/arch/arm/plat-omap/devices.c
+++ b/arch/arm/plat-omap/devices.c
@@ -72,6 +72,40 @@ void omap_mcbsp_register_board_cfg(struct omap_mcbsp_platform_data *config,
 
 /*-------------------------------------------------------------------------*/
 
+#if defined(CONFIG_SND_OMAP_SOC_DMIC) || \
+	defined(CONFIG_SND_OMAP_SOC_DMIC_MODULE)
+
+static struct omap_device_pm_latency omap_dmic_latency[] = {
+	{
+		.deactivate_func = omap_device_idle_hwmods,
+		.activate_func = omap_device_enable_hwmods,
+		.flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
+	},
+};
+
+static void omap_init_dmic(void)
+{
+	struct omap_hwmod *oh;
+	struct omap_device *od;
+
+	oh = omap_hwmod_lookup("omap-dmic");
+	if (!oh) {
+		printk(KERN_ERR "Could not look up dmic hw_mod\n");
+		return;
+	}
+
+	od = omap_device_build("omap-dmic-dai", -1, oh, NULL, 0,
+				omap_dmic_latency,
+				ARRAY_SIZE(omap_dmic_latency), 0);
+	if (IS_ERR(od))
+		printk(KERN_ERR "Could not build omap_device for omap-dmic-dai\n");
+}
+#else
+static inline void omap_init_dmic(void) {}
+#endif
+
+/*-------------------------------------------------------------------------*/
+
 #if defined(CONFIG_SND_OMAP_SOC_MCPDM) || \
 		defined(CONFIG_SND_OMAP_SOC_MCPDM_MODULE)
 
@@ -328,6 +362,7 @@ static int __init omap_init_devices(void)
 	/* please keep these calls, and their implementations above,
 	 * in alphabetical order so they're easier to sort through.
 	 */
+	omap_init_dmic();
 	omap_init_rng();
 	omap_init_mcpdm();
 	omap_init_uwire();
-- 
1.7.0.4

[PATCH 5/5] MAP4: DMIC: Add DMIC codec platform devices

[David Lambert]

This creates the DMIC codec platform devices.

The platform devices create an instance of the driver during boot up.

Signed-off-by: David Lambert <dlambert@ti.com>
---
 arch/arm/mach-omap2/board-4430sdp.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
index df5a425..bf271e5 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -505,6 +505,16 @@ static void __init omap_sfh7741prox_init(void)
 	}
 }
 
+static struct platform_device codec_dmic0 = {
+	.name   = "dmic-codec",
+	.id     = 0,
+};
+
+static inline void omap_dmic_init(void)
+{
+	platform_device_register(&codec_dmic0);
+}
+
 static void __init omap_4430sdp_init(void)
 {
 	int status;
@@ -528,6 +538,7 @@ static void __init omap_4430sdp_init(void)
 		spi_register_board_info(sdp4430_spi_board_info,
 				ARRAY_SIZE(sdp4430_spi_board_info));
 	}
+	omap_dmic_init();
 }
 
 static void __init omap_4430sdp_map_io(void)
-- 
1.7.0.4

Re: [PATCH 5/5] MAP4: DMIC: Add DMIC codec platform devices

[Felipe Balbi]

Hi,

$SUBJECT should read OMAP4 instead of MAP4

On Mon, Dec 27, 2010 at 10:17:06PM -0600, David Lambert wrote:
>This creates the DMIC codec platform devices.
>
>The platform devices create an instance of the driver during boot up.
>
>Signed-off-by: David Lambert <dlambert@ti.com>

is this a "virtual" device ? If not, where's HWMOD for it ?

>diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
>index df5a425..bf271e5 100644
>--- a/arch/arm/mach-omap2/board-4430sdp.c
>+++ b/arch/arm/mach-omap2/board-4430sdp.c
>@@ -505,6 +505,16 @@ static void __init omap_sfh7741prox_init(void)
> 	}
> }
>
>+static struct platform_device codec_dmic0 = {
>+	.name   = "dmic-codec",
>+	.id     = 0,

if this is the only device, should id be -1 instead ?

>+};
>+
>+static inline void omap_dmic_init(void)
>+{
>+	platform_device_register(&codec_dmic0);

Another option would be to use
platform_device_register_simple("dmi-codec", -1);

-- 
balbi