All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Support for OMAP4 Digital Microphone interface
@ 2011-11-22 14:01 Peter Ujfalusi
  2011-11-22 14:01 ` [PATCH 1/5] OMAP4: hwmod: Add names for DMIC memory address space Peter Ujfalusi
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Peter Ujfalusi @ 2011-11-22 14:01 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Tony Lindgren
  Cc: alsa-devel, linux-omap, Benoit Cousson

Hello,

The following series will add support for OMAP4 DMIC interface, and enable them
on sdp4430/Blaze boards.

Mark: The Panda support will be based on this series with the conversion of the
sdp4430 ASoC machine driver to platform device/driver to prepare it for DT.
The DT bindings for the dmic driver will be sent shortly as well.

Regards,
Peter
---
Peter Ujfalusi (5):
  OMAP4: hwmod: Add names for DMIC memory address space
  ASoC: OMAP4: omap-dmic: Initial support for OMAP DMIC
  OMAP4: devices: Register OMAP4 DMIC platform device
  OMAP4: board-4430sdp: Register platform device for digimic codec
  ASoC: sdp4430: Add support for digital microphones

 arch/arm/mach-omap2/board-4430sdp.c        |    6 +
 arch/arm/mach-omap2/devices.c              |   22 ++
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    2 +
 sound/soc/omap/Kconfig                     |    5 +
 sound/soc/omap/Makefile                    |    2 +
 sound/soc/omap/omap-dmic.c                 |  494 ++++++++++++++++++++++++++++
 sound/soc/omap/omap-dmic.h                 |   73 ++++
 sound/soc/omap/sdp4430.c                   |   84 ++++-
 8 files changed, 677 insertions(+), 11 deletions(-)
 create mode 100644 sound/soc/omap/omap-dmic.c
 create mode 100644 sound/soc/omap/omap-dmic.h

-- 
1.7.8.rc3


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/5] OMAP4: hwmod: Add names for DMIC memory address space
  2011-11-22 14:01 [PATCH 0/5] Support for OMAP4 Digital Microphone interface Peter Ujfalusi
@ 2011-11-22 14:01 ` Peter Ujfalusi
  2011-11-22 14:01 ` [PATCH 2/5] ASoC: OMAP4: omap-dmic: Initial support for OMAP DMIC Peter Ujfalusi
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Peter Ujfalusi @ 2011-11-22 14:01 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Tony Lindgren
  Cc: alsa-devel, linux-omap, Benoit Cousson

To be able to get the memory resources by name from
the DMIC driver (for MPU and for DMA).

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    2 ++
 1 files changed, 2 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 7695e5d..8b75c60 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -1028,6 +1028,7 @@ static struct omap_hwmod_dma_info omap44xx_dmic_sdma_reqs[] = {
 
 static struct omap_hwmod_addr_space omap44xx_dmic_addrs[] = {
 	{
+		.name		= "mpu",
 		.pa_start	= 0x4012e000,
 		.pa_end		= 0x4012e07f,
 		.flags		= ADDR_TYPE_RT
@@ -1046,6 +1047,7 @@ static struct omap_hwmod_ocp_if omap44xx_l4_abe__dmic = {
 
 static struct omap_hwmod_addr_space omap44xx_dmic_dma_addrs[] = {
 	{
+		.name		= "dma",
 		.pa_start	= 0x4902e000,
 		.pa_end		= 0x4902e07f,
 		.flags		= ADDR_TYPE_RT
-- 
1.7.8.rc3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/5] ASoC: OMAP4: omap-dmic: Initial support for OMAP DMIC
  2011-11-22 14:01 [PATCH 0/5] Support for OMAP4 Digital Microphone interface Peter Ujfalusi
  2011-11-22 14:01 ` [PATCH 1/5] OMAP4: hwmod: Add names for DMIC memory address space Peter Ujfalusi
@ 2011-11-22 14:01 ` Peter Ujfalusi
  2011-11-22 16:01   ` Mark Brown
  2011-11-22 14:01 ` [PATCH 3/5] OMAP4: devices: Register OMAP4 DMIC platform device Peter Ujfalusi
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Peter Ujfalusi @ 2011-11-22 14:01 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Tony Lindgren
  Cc: alsa-devel, linux-omap, Benoit Cousson

Add support for OMAP4 Digital Microphone interface.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/omap/Kconfig     |    3 +
 sound/soc/omap/Makefile    |    2 +
 sound/soc/omap/omap-dmic.c |  494 ++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/omap/omap-dmic.h |   73 +++++++
 4 files changed, 572 insertions(+), 0 deletions(-)
 create mode 100644 sound/soc/omap/omap-dmic.c
 create mode 100644 sound/soc/omap/omap-dmic.h

diff --git a/sound/soc/omap/Kconfig b/sound/soc/omap/Kconfig
index fe83d0d..052254a 100644
--- a/sound/soc/omap/Kconfig
+++ b/sound/soc/omap/Kconfig
@@ -2,6 +2,9 @@ config SND_OMAP_SOC
 	tristate "SoC Audio for the Texas Instruments OMAP chips"
 	depends on ARCH_OMAP
 
+config SND_OMAP_SOC_DMIC
+	tristate
+
 config SND_OMAP_SOC_MCBSP
 	tristate
 	select OMAP_MCBSP
diff --git a/sound/soc/omap/Makefile b/sound/soc/omap/Makefile
index 052fd75..1fd723f 100644
--- a/sound/soc/omap/Makefile
+++ b/sound/soc/omap/Makefile
@@ -1,10 +1,12 @@
 # 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
 snd-soc-omap-hdmi-objs := omap-hdmi.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
 obj-$(CONFIG_SND_OMAP_SOC_HDMI) += snd-soc-omap-hdmi.o
diff --git a/sound/soc/omap/omap-dmic.c b/sound/soc/omap/omap-dmic.c
new file mode 100644
index 0000000..5c7e6ca
--- /dev/null
+++ b/sound/soc/omap/omap-dmic.c
@@ -0,0 +1,494 @@
+/*
+ * omap-dmic.c  --  OMAP ASoC DMIC DAI driver
+ *
+ * Copyright (C) 2010 - 2011 Texas Instruments
+ *
+ * Author: David Lambert <dlambert@ti.com>
+ *	   Misael Lopez Cruz <misael.lopez@ti.com>
+ *	   Liam Girdwood <lrg@ti.com>
+ *	   Peter Ujfalusi <peter.ujfalusi@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/pm_runtime.h>
+#include <plat/dma.h>
+
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/initval.h>
+#include <sound/soc.h>
+
+#include "omap-pcm.h"
+#include "omap-dmic.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 clk_freq;
+	int sysclk;
+	int threshold;
+	int abe_mode;
+	u32 ch_enabled;
+	struct mutex mutex;
+};
+
+/*
+ * 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,
+};
+
+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);
+}
+
+static inline void omap_dmic_start(struct omap_dmic *dmic)
+{
+	u32 ctrl = omap_dmic_read(dmic, OMAP_DMIC_CTRL_REG);
+
+	/* Configure DMA controller */
+	omap_dmic_write(dmic, OMAP_DMIC_DMAENABLE_SET_REG,
+			OMAP_DMIC_DMA_ENABLE);
+
+	omap_dmic_write(dmic, OMAP_DMIC_CTRL_REG, ctrl | dmic->ch_enabled);
+}
+
+static inline void omap_dmic_stop(struct omap_dmic *dmic)
+{
+	u32 ctrl = omap_dmic_read(dmic, OMAP_DMIC_CTRL_REG);
+	omap_dmic_write(dmic, OMAP_DMIC_CTRL_REG,
+			ctrl & ~OMAP_DMIC_UP_ENABLE_MASK);
+
+	/* Disable DMA request generation */
+	omap_dmic_write(dmic, OMAP_DMIC_DMAENABLE_CLR_REG,
+			OMAP_DMIC_DMA_ENABLE);
+
+}
+
+static inline int dmic_is_enabled(struct omap_dmic *dmic)
+{
+	return omap_dmic_read(dmic, OMAP_DMIC_CTRL_REG) &
+						OMAP_DMIC_UP_ENABLE_MASK;
+}
+
+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);
+	int ret = 0;
+
+	mutex_lock(&dmic->mutex);
+
+	if (!dai->active)
+		pm_runtime_get_sync(dmic->dev);
+	else
+		ret = -EBUSY;
+
+	mutex_unlock(&dmic->mutex);
+	return ret;
+}
+
+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);
+
+	mutex_lock(&dmic->mutex);
+
+	if (!dai->active)
+		pm_runtime_put_sync(dmic->dev);
+
+	mutex_unlock(&dmic->mutex);
+}
+
+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);
+	int channels;
+
+	dmic->ch_enabled = 0;
+	channels = params_channels(params);
+	switch (channels) {
+	case 6:
+		dmic->ch_enabled |= OMAP_DMIC_UP3_ENABLE;
+	case 4:
+		dmic->ch_enabled |= OMAP_DMIC_UP2_ENABLE;
+	case 2:
+		dmic->ch_enabled |= OMAP_DMIC_UP1_ENABLE;
+		break;
+	default:
+		dev_err(dmic->dev, "invalid number of legacy channels\n");
+		return -EINVAL;
+	}
+
+	switch (params_rate(params)) {
+	case 96000:
+	case 192000:
+		break;
+	default:
+		dev_err(dmic->dev, "rate %d not supported\n",
+			params_rate(params));
+		return -EINVAL;
+	}
+
+	/* packet size is threshold * channels */
+	omap_dmic_dai_dma_params.packet_size = dmic->threshold * channels;
+	snd_soc_dai_set_dma_data(dai, substream, &omap_dmic_dai_dma_params);
+
+	return 0;
+}
+
+static int omap_dmic_dai_prepare(struct snd_pcm_substream *substream,
+				  struct snd_soc_dai *dai)
+{
+	struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
+	u32 ctrl;
+
+	/* Configure uplink threshold */
+	omap_dmic_write(dmic, OMAP_DMIC_FIFO_CTRL_REG, dmic->threshold);
+
+	/* Set dmic out format */
+	ctrl = omap_dmic_read(dmic, OMAP_DMIC_CTRL_REG)
+		& ~(OMAP_DMIC_FORMAT | OMAP_DMIC_POLAR_MASK);
+	omap_dmic_write(dmic, OMAP_DMIC_CTRL_REG,
+			ctrl | OMAP_DMICOUTFORMAT_LJUST |
+			OMAP_DMIC_POLAR1 | OMAP_DMIC_POLAR2 | OMAP_DMIC_POLAR3);
+
+	return 0;
+}
+
+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);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		omap_dmic_start(dmic);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+		omap_dmic_stop(dmic);
+		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;
+	char *parent_clk_name;
+	int ret = 0;
+
+	if (dmic->sysclk == clk_id) {
+		dmic->clk_freq = freq;
+		return 0;
+	}
+
+	/* re-parent not allowed if a stream is ongoing */
+	if (dmic_is_enabled(dmic)) {
+		dev_err(dmic->dev, "can't re-parent when DMIC active\n");
+		return -EBUSY;
+	}
+
+	dmic_clk = clk_get(dmic->dev, "dmic_fck");
+	if (IS_ERR(dmic_clk)) {
+		dev_err(dmic->dev, "cant get dmic_fck\n");
+		return -ENODEV;
+	}
+
+	switch (clk_id) {
+	case OMAP_DMIC_SYSCLK_PAD_CLKS:
+		parent_clk_name = "pad_clks_ck";
+		break;
+	case OMAP_DMIC_SYSCLK_SLIMBLUS_CLKS:
+		parent_clk_name = "slimbus_clk";
+		break;
+	case OMAP_DMIC_SYSCLK_SYNC_MUX_CLKS:
+		parent_clk_name = "dmic_sync_mux_ck";
+		break;
+	default:
+		dev_err(dai->dev, "clk_id not supported %d\n", clk_id);
+		ret = -EINVAL;
+		goto err_par;
+	}
+
+	parent_clk = clk_get(dmic->dev, parent_clk_name);
+	if (IS_ERR(parent_clk)) {
+		dev_err(dmic->dev, "can't get %s\n", parent_clk_name);
+		ret = -ENODEV;
+		goto err_par;
+	}
+
+	/* disable clock while reparenting */
+	pm_runtime_put_sync(dmic->dev);
+	ret = clk_set_parent(dmic_clk, parent_clk);
+	pm_runtime_get_sync(dmic->dev);
+	if (ret < 0) {
+		dev_err(dmic->dev, "re-parent failed\n");
+		goto err_busy;
+	}
+
+	dmic->sysclk = clk_id;
+	dmic->clk_freq = freq;
+
+err_busy:
+	clk_put(parent_clk);
+err_par:
+	clk_put(dmic_clk);
+
+	return ret;
+}
+
+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 div_sel;
+	u32 ctrl;
+
+	if (div_id != OMAP_DMIC_CLKDIV)
+		return -ENODEV;
+
+	switch (dmic->clk_freq) {
+	case 19200000:
+		switch (div) {
+		case 5:
+			div_sel = 0x1;
+			break;
+		case 8:
+			div_sel = 0x0;
+			break;
+		default:
+			goto div_err;
+		}
+		break;
+	case 24000000:
+		switch (div) {
+		case 10:
+			div_sel = 0x2;
+			break;
+		default:
+			goto div_err;
+		}
+		break;
+	case 24576000:
+		switch (div) {
+		case 8:
+			div_sel = 0x3;
+			break;
+		case 16:
+			div_sel = 0x4;
+			break;
+		default:
+			goto div_err;
+		}
+		break;
+	case 12000000:
+		switch (div) {
+		case 5:
+			div_sel = 0x5;
+			break;
+		default:
+			goto div_err;
+		}
+		break;
+	default:
+		dev_err(dai->dev, "invalid freq %d\n", dmic->clk_freq);
+		return -EINVAL;
+	}
+
+	ctrl = omap_dmic_read(dmic, OMAP_DMIC_CTRL_REG);
+	ctrl &= ~OMAP_DMIC_CLK_DIV_MASK;
+	omap_dmic_write(dmic, OMAP_DMIC_CTRL_REG,
+			ctrl | OMAP_DMIC_CLK_DIV(div_sel));
+
+	return 0;
+div_err:
+	dev_err(dai->dev, "invalid divider (%d) for %dHz", div, dmic->clk_freq);
+	return -EINVAL;
+}
+
+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,
+	.prepare	= omap_dmic_dai_prepare,
+	.trigger	= omap_dmic_dai_trigger,
+	.set_sysclk	= omap_dmic_set_dai_sysclk,
+	.set_clkdiv	= omap_dmic_set_clkdiv,
+};
+
+static int omap_dmic_probe(struct snd_soc_dai *dai)
+{
+	struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
+
+	pm_runtime_enable(dmic->dev);
+
+	/* Disable lines while request is ongoing */
+	pm_runtime_get_sync(dmic->dev);
+	omap_dmic_write(dmic, OMAP_DMIC_CTRL_REG, 0x00);
+	pm_runtime_put_sync(dmic->dev);
+
+	/* Configure DMIC threshold value */
+	dmic->threshold = OMAP_DMIC_THRES_MAX - 3;
+	return 0;
+}
+
+static int omap_dmic_remove(struct snd_soc_dai *dai)
+{
+	struct omap_dmic *dmic = snd_soc_dai_get_drvdata(dai);
+
+	pm_runtime_disable(dmic->dev);
+
+	return 0;
+}
+
+static struct snd_soc_dai_driver omap_dmic_dai = {
+	.name = "omap-dmic",
+	.probe = omap_dmic_probe,
+	.remove = omap_dmic_remove,
+	.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->sysclk = OMAP_DMIC_SYSCLK_SYNC_MUX_CLKS;
+
+	mutex_init(&dmic->mutex);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dma");
+	if (!res) {
+		dev_err(dmic->dev, "invalid dma memory resource\n");
+		ret = -ENODEV;
+		goto err_res;
+	}
+	omap_dmic_dai_dma_params.port_addr = res->start + OMAP_DMIC_DATA_REG;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mpu");
+	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;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_DMA, 0);
+	if (!res) {
+		dev_err(dmic->dev, "invalid dma resource\n");
+		ret = -ENODEV;
+		goto err_dai;
+	}
+	omap_dmic_dai_dma_params.dma_req = res->start;
+
+	ret = snd_soc_register_dai(&pdev->dev, &omap_dmic_dai);
+	if (ret)
+		goto err_dai;
+
+	return 0;
+
+err_dai:
+	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);
+	iounmap(dmic->io_base);
+	kfree(dmic);
+
+	return 0;
+}
+
+static struct platform_driver asoc_dmic_driver = {
+	.driver = {
+		.name = "omap-dmic",
+		.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_ALIAS("platform:omap-dmic");
+MODULE_AUTHOR("Peter Ujfalusi <peter.ujfalusi@ti.com>");
+MODULE_DESCRIPTION("OMAP DMIC ASoC 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..3f964d8
--- /dev/null
+++ b/sound/soc/omap/omap-dmic.h
@@ -0,0 +1,73 @@
+/*
+ * omap-dmic.h  --  OMAP Digital Microphone Controller
+ *
+ * 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
+
+#define OMAP_DMIC_REVISION_REG		0x00
+#define OMAP_DMIC_SYSCONFIG_REG		0x10
+#define OMAP_DMIC_IRQSTATUS_RAW_REG	0x24
+#define OMAP_DMIC_IRQSTATUS_REG		0x28
+#define OMAP_DMIC_IRQENABLE_SET_REG	0x2C
+#define OMAP_DMIC_IRQENABLE_CLR_REG	0x30
+#define OMAP_DMIC_IRQWAKE_EN_REG	0x34
+#define OMAP_DMIC_DMAENABLE_SET_REG	0x38
+#define OMAP_DMIC_DMAENABLE_CLR_REG	0x3C
+#define OMAP_DMIC_DMAWAKEEN_REG		0x40
+#define OMAP_DMIC_CTRL_REG		0x44
+#define OMAP_DMIC_DATA_REG		0x48
+#define OMAP_DMIC_FIFO_CTRL_REG		0x4C
+#define OMAP_DMIC_FIFO_DMIC1R_DATA_REG	0x50
+#define OMAP_DMIC_FIFO_DMIC1L_DATA_REG	0x54
+#define OMAP_DMIC_FIFO_DMIC2R_DATA_REG	0x58
+#define OMAP_DMIC_FIFO_DMIC2L_DATA_REG	0x5C
+#define OMAP_DMIC_FIFO_DMIC3R_DATA_REG	0x60
+#define OMAP_DMIC_FIFO_DMIC3L_DATA_REG	0x64
+
+/* IRQSTATUS_RAW, IRQSTATUS, IRQENABLE_SET, IRQENABLE_CLR bit fields */
+#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		(1 << 0)
+#define OMAP_DMIC_UP2_ENABLE		(1 << 1)
+#define OMAP_DMIC_UP3_ENABLE		(1 << 2)
+#define OMAP_DMIC_UP_ENABLE_MASK	0x7
+#define OMAP_DMIC_FORMAT		(1 << 3)
+#define OMAP_DMIC_POLAR1		(1 << 4)
+#define OMAP_DMIC_POLAR2		(1 << 5)
+#define OMAP_DMIC_POLAR3		(1 << 6)
+#define OMAP_DMIC_POLAR_MASK		(0x7 << 4)
+#define OMAP_DMIC_CLK_DIV(x)		(((x) & 0x7) << 7)
+#define OMAP_DMIC_CLK_DIV_MASK		(0x7 << 7)
+#define	OMAP_DMIC_RESET			(1 << 10)
+
+#define OMAP_DMICOUTFORMAT_LJUST	(0 << 3)
+#define OMAP_DMICOUTFORMAT_RJUST	(1 << 3)
+
+/* DMIC_FIFO_CTRL bit fields */
+#define OMAP_DMIC_THRES_MAX		0xF
+
+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,
+};
+
+#endif
-- 
1.7.8.rc3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/5] OMAP4: devices: Register OMAP4 DMIC platform device
  2011-11-22 14:01 [PATCH 0/5] Support for OMAP4 Digital Microphone interface Peter Ujfalusi
  2011-11-22 14:01 ` [PATCH 1/5] OMAP4: hwmod: Add names for DMIC memory address space Peter Ujfalusi
  2011-11-22 14:01 ` [PATCH 2/5] ASoC: OMAP4: omap-dmic: Initial support for OMAP DMIC Peter Ujfalusi
@ 2011-11-22 14:01 ` Peter Ujfalusi
  2011-11-22 14:01 ` [PATCH 4/5] OMAP4: board-4430sdp: Register platform device for digimic codec Peter Ujfalusi
  2011-11-22 14:02 ` [PATCH 5/5] ASoC: sdp4430: Add support for digital microphones Peter Ujfalusi
  4 siblings, 0 replies; 13+ messages in thread
From: Peter Ujfalusi @ 2011-11-22 14:01 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Tony Lindgren
  Cc: alsa-devel, linux-omap, Benoit Cousson

Add platform device registration for OMAP4 DMIC.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 arch/arm/mach-omap2/devices.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index c15cfad..35d5dff 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -336,6 +336,27 @@ static void omap_init_mcpdm(void)
 static inline void omap_init_mcpdm(void) {}
 #endif
 
+#if defined(CONFIG_SND_OMAP_SOC_DMIC) || \
+		defined(CONFIG_SND_OMAP_SOC_DMIC_MODULE)
+
+static void omap_init_dmic(void)
+{
+	struct omap_hwmod *oh;
+	struct platform_device *pdev;
+
+	oh = omap_hwmod_lookup("dmic");
+	if (!oh) {
+		printk(KERN_ERR "Could not look up mcpdm hw_mod\n");
+		return;
+	}
+
+	pdev = omap_device_build("omap-dmic", -1, oh, NULL, 0, NULL, 0, 0);
+	WARN(IS_ERR(pdev), "Can't build omap_device for omap-dmic.\n");
+}
+#else
+static inline void omap_init_dmic(void) {}
+#endif
+
 #if defined(CONFIG_SPI_OMAP24XX) || defined(CONFIG_SPI_OMAP24XX_MODULE)
 
 #include <plat/mcspi.h>
@@ -681,6 +702,7 @@ static int __init omap2_init_devices(void)
 	 */
 	omap_init_audio();
 	omap_init_mcpdm();
+	omap_init_dmic();
 	omap_init_camera();
 	omap_init_mbox();
 	omap_init_mcspi();
-- 
1.7.8.rc3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/5] OMAP4: board-4430sdp: Register platform device for digimic codec
  2011-11-22 14:01 [PATCH 0/5] Support for OMAP4 Digital Microphone interface Peter Ujfalusi
                   ` (2 preceding siblings ...)
  2011-11-22 14:01 ` [PATCH 3/5] OMAP4: devices: Register OMAP4 DMIC platform device Peter Ujfalusi
@ 2011-11-22 14:01 ` Peter Ujfalusi
  2011-11-22 14:02 ` [PATCH 5/5] ASoC: sdp4430: Add support for digital microphones Peter Ujfalusi
  4 siblings, 0 replies; 13+ messages in thread
From: Peter Ujfalusi @ 2011-11-22 14:01 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Tony Lindgren
  Cc: alsa-devel, linux-omap, Benoit Cousson

OMAP4 SDP/Blaze boards have onboard digital microphones.
Register the platform device for the dmic-codec to be
able to use the microphones.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 arch/arm/mach-omap2/board-4430sdp.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
index 5156468..503bf28 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -372,11 +372,17 @@ static struct platform_device sdp4430_vbat = {
 	},
 };
 
+static struct platform_device sdp4430_dmic_codec = {
+	.name	= "dmic-codec",
+	.id	= -1,
+};
+
 static struct platform_device *sdp4430_devices[] __initdata = {
 	&sdp4430_gpio_keys_device,
 	&sdp4430_leds_gpio,
 	&sdp4430_leds_pwm,
 	&sdp4430_vbat,
+	&sdp4430_dmic_codec,
 };
 
 static struct omap_musb_board_data musb_board_data = {
-- 
1.7.8.rc3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/5] ASoC: sdp4430: Add support for digital microphones
  2011-11-22 14:01 [PATCH 0/5] Support for OMAP4 Digital Microphone interface Peter Ujfalusi
                   ` (3 preceding siblings ...)
  2011-11-22 14:01 ` [PATCH 4/5] OMAP4: board-4430sdp: Register platform device for digimic codec Peter Ujfalusi
@ 2011-11-22 14:02 ` Peter Ujfalusi
  4 siblings, 0 replies; 13+ messages in thread
From: Peter Ujfalusi @ 2011-11-22 14:02 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Tony Lindgren
  Cc: alsa-devel, linux-omap, Benoit Cousson

OMAP4 SDP/Blaze boards have digital microphones.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/omap/Kconfig   |    2 +
 sound/soc/omap/sdp4430.c |   84 ++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 75 insertions(+), 11 deletions(-)

diff --git a/sound/soc/omap/Kconfig b/sound/soc/omap/Kconfig
index 052254a..fb1bf25 100644
--- a/sound/soc/omap/Kconfig
+++ b/sound/soc/omap/Kconfig
@@ -100,8 +100,10 @@ config SND_OMAP_SOC_SDP3430
 config SND_OMAP_SOC_SDP4430
 	tristate "SoC Audio support for Texas Instruments SDP4430"
 	depends on TWL4030_CORE && SND_OMAP_SOC && MACH_OMAP_4430SDP
+	select SND_OMAP_SOC_DMIC
 	select SND_OMAP_SOC_MCPDM
 	select SND_SOC_TWL6040
+	select SND_SOC_DMIC
 	help
 	  Say Y if you want to add support for SoC audio on Texas Instruments
 	  SDP4430.
diff --git a/sound/soc/omap/sdp4430.c b/sound/soc/omap/sdp4430.c
index 03d9fa4..a8c4926 100644
--- a/sound/soc/omap/sdp4430.c
+++ b/sound/soc/omap/sdp4430.c
@@ -33,6 +33,7 @@
 #include <plat/hardware.h>
 #include <plat/mux.h>
 
+#include "omap-dmic.h"
 #include "omap-mcpdm.h"
 #include "omap-pcm.h"
 #include "../codecs/twl6040.h"
@@ -67,6 +68,31 @@ static struct snd_soc_ops sdp4430_ops = {
 	.hw_params = sdp4430_hw_params,
 };
 
+static int sdp4430_dmic_hw_params(struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	int ret = 0;
+
+	ret = snd_soc_dai_set_sysclk(cpu_dai, OMAP_DMIC_SYSCLK_PAD_CLKS,
+				     19200000, SND_SOC_CLOCK_IN);
+	if (ret < 0) {
+		printk(KERN_ERR "can't set DMIC cpu system clock\n");
+		return ret;
+	}
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, OMAP_DMIC_CLKDIV, 8);
+	if (ret < 0) {
+		printk(KERN_ERR "can't set DMIC cpu clock divider\n");
+		return ret;
+	}
+	return 0;
+}
+
+static struct snd_soc_ops sdp4430_dmic_ops = {
+	.hw_params = sdp4430_dmic_hw_params,
+};
+
 /* Headset jack */
 static struct snd_soc_jack hs_jack;
 
@@ -148,23 +174,59 @@ static int sdp4430_twl6040_init(struct snd_soc_pcm_runtime *rtd)
 	return ret;
 }
 
+static const struct snd_soc_dapm_widget sdp4430_dmic_dapm_widgets[] = {
+	SND_SOC_DAPM_MIC("Digital Mic", NULL),
+};
+
+static const struct snd_soc_dapm_route dmic_audio_map[] = {
+	{"DMic", NULL, "Digital Mic1 Bias"},
+	{"Digital Mic1 Bias", NULL, "Digital Mic"},
+};
+
+static int sdp4430_dmic_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_codec *codec = rtd->codec;
+	struct snd_soc_dapm_context *dapm = &codec->dapm;
+	int ret;
+
+	ret = snd_soc_dapm_new_controls(dapm, sdp4430_dmic_dapm_widgets,
+				ARRAY_SIZE(sdp4430_dmic_dapm_widgets));
+	if (ret)
+		return ret;
+
+	return snd_soc_dapm_add_routes(dapm, dmic_audio_map,
+				ARRAY_SIZE(dmic_audio_map));
+}
+
 /* Digital audio interface glue - connects codec <--> CPU */
-static struct snd_soc_dai_link sdp4430_dai = {
-	.name = "TWL6040",
-	.stream_name = "TWL6040",
-	.cpu_dai_name = "omap-mcpdm",
-	.codec_dai_name = "twl6040-legacy",
-	.platform_name = "omap-pcm-audio",
-	.codec_name = "twl6040-codec",
-	.init = sdp4430_twl6040_init,
-	.ops = &sdp4430_ops,
+static struct snd_soc_dai_link sdp4430_dai[] = {
+	{
+		.name = "TWL6040",
+		.stream_name = "TWL6040",
+		.cpu_dai_name = "omap-mcpdm",
+		.codec_dai_name = "twl6040-legacy",
+		.platform_name = "omap-pcm-audio",
+		.codec_name = "twl6040-codec",
+		.init = sdp4430_twl6040_init,
+		.ops = &sdp4430_ops,
+	},
+	{
+		.name = "DMIC",
+		.stream_name = "DMIC Capture",
+		.cpu_dai_name = "omap-dmic",
+		.codec_dai_name = "dmic-hifi",
+		.platform_name = "omap-pcm-audio",
+		.codec_name = "dmic-codec",
+		.init = sdp4430_dmic_init,
+		.ops = &sdp4430_dmic_ops,
+	},
 };
 
 /* Audio machine driver */
 static struct snd_soc_card snd_soc_sdp4430 = {
 	.name = "SDP4430",
-	.dai_link = &sdp4430_dai,
-	.num_links = 1,
+	.dai_link = sdp4430_dai,
+	.num_links = ARRAY_SIZE(sdp4430_dai),
 
 	.dapm_widgets = sdp4430_twl6040_dapm_widgets,
 	.num_dapm_widgets = ARRAY_SIZE(sdp4430_twl6040_dapm_widgets),
-- 
1.7.8.rc3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/5] ASoC: OMAP4: omap-dmic: Initial support for OMAP DMIC
  2011-11-22 14:01 ` [PATCH 2/5] ASoC: OMAP4: omap-dmic: Initial support for OMAP DMIC Peter Ujfalusi
@ 2011-11-22 16:01   ` Mark Brown
  2011-11-23  8:48     ` Péter Ujfalusi
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2011-11-22 16:01 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Liam Girdwood, Tony Lindgren, alsa-devel, linux-omap, Benoit Cousson

On Tue, Nov 22, 2011 at 04:01:57PM +0200, Peter Ujfalusi wrote:

> +	switch (params_rate(params)) {
> +	case 96000:
> +	case 192000:
> +		break;

Why doesn't the driver need to tell the hardware what sample rate to run
at?

> +	dmic_clk = clk_get(dmic->dev, "dmic_fck");
> +	if (IS_ERR(dmic_clk)) {
> +		dev_err(dmic->dev, "cant get dmic_fck\n");
> +		return -ENODEV;
> +	}

Why aren't we getting and holding a reference to the clock over the
entire lifetime of the driver?

> +	/* disable clock while reparenting */
> +	pm_runtime_put_sync(dmic->dev);
> +	ret = clk_set_parent(dmic_clk, parent_clk);
> +	pm_runtime_get_sync(dmic->dev);

Since we're only allowing reclocking while idle shouldn't the clock
already be disabled?  Seems like it ought to be good for power if
nothing else...

> +static int omap_dmic_set_clkdiv(struct snd_soc_dai *dai,
> +				int div_id, int div)
> +{

DMIC clocking is usually fairly simple so it seems surprising that the
driver isn't able to figure this out for itself.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Re: [PATCH 2/5] ASoC: OMAP4: omap-dmic: Initial support for OMAP DMIC
  2011-11-22 16:01   ` Mark Brown
@ 2011-11-23  8:48     ` Péter Ujfalusi
  2011-11-23 10:58       ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: Péter Ujfalusi @ 2011-11-23  8:48 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Tony Lindgren, alsa-devel, linux-omap, Benoit Cousson

On Tuesday 22 November 2011 16:01:05 Mark Brown wrote:
> On Tue, Nov 22, 2011 at 04:01:57PM +0200, Peter Ujfalusi wrote:
> > +	switch (params_rate(params)) {
> > +	case 96000:
> > +	case 192000:
> > +		break;
> 
> Why doesn't the driver need to tell the hardware what sample rate to run
> at?

Because the OMAP4 DMIC can only support on 96KHz...
Will be fixed.

> 
> > +	dmic_clk = clk_get(dmic->dev, "dmic_fck");
> > +	if (IS_ERR(dmic_clk)) {
> > +		dev_err(dmic->dev, "cant get dmic_fck\n");
> > +		return -ENODEV;
> > +	}
> 
> Why aren't we getting and holding a reference to the clock over the
> entire lifetime of the driver?

We only need the reference for the dmic_fclk for reparenting which will happen 
only once in most cases (or not at all, if pad_clks_ck is going to be used). I 
don't think we should hold the reference for the dmic_fclk.
The clock handling is done via pm_runtime_get/put_sync().

> > +	/* disable clock while reparenting */
> > +	pm_runtime_put_sync(dmic->dev);
> > +	ret = clk_set_parent(dmic_clk, parent_clk);
> > +	pm_runtime_get_sync(dmic->dev);
> 
> Since we're only allowing reclocking while idle shouldn't the clock
> already be disabled?  Seems like it ought to be good for power if
> nothing else...

We enable the clocks at dai_startup for the DMIC (and disable them on 
dai_shutdown). We can not reparent while the clocks are enabled.
This is the reason for this part.
 
> > +static int omap_dmic_set_clkdiv(struct snd_soc_dai *dai,
> > +				int div_id, int div)
> > +{
> 
> DMIC clocking is usually fairly simple so it seems surprising that the
> driver isn't able to figure this out for itself.

The clock towards the external digital mics are based on the DMIC_FCLK rate.
In case of DMIC_FCLK = 19.2MHz, 24.576MHz we can select between two dividers 
which will result different clocks for the external microphones.
I would rather leave this decision to the machine driver which knows the 
external components, and can pick the best divider for them.

--
Péter
--
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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Re: [PATCH 2/5] ASoC: OMAP4: omap-dmic: Initial support for OMAP DMIC
  2011-11-23  8:48     ` Péter Ujfalusi
@ 2011-11-23 10:58       ` Mark Brown
  2011-11-23 14:00         ` Péter Ujfalusi
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2011-11-23 10:58 UTC (permalink / raw)
  To: Péter Ujfalusi
  Cc: Liam Girdwood, Tony Lindgren, alsa-devel, linux-omap, Benoit Cousson

On Wed, Nov 23, 2011 at 10:48:24AM +0200, Péter Ujfalusi wrote:
> On Tuesday 22 November 2011 16:01:05 Mark Brown wrote:

> > > +	dmic_clk = clk_get(dmic->dev, "dmic_fck");
> > > +	if (IS_ERR(dmic_clk)) {
> > > +		dev_err(dmic->dev, "cant get dmic_fck\n");
> > > +		return -ENODEV;
> > > +	}

> > Why aren't we getting and holding a reference to the clock over the
> > entire lifetime of the driver?

> We only need the reference for the dmic_fclk for reparenting which will happen 
> only once in most cases (or not at all, if pad_clks_ck is going to be used). I 
> don't think we should hold the reference for the dmic_fclk.
> The clock handling is done via pm_runtime_get/put_sync().

This just seems like it's making the code needlessly complex - there's
no harm in holding the reference if you don't enable/disable the clock
and it makes this function much simpler.

> > > +	/* disable clock while reparenting */
> > > +	pm_runtime_put_sync(dmic->dev);
> > > +	ret = clk_set_parent(dmic_clk, parent_clk);
> > > +	pm_runtime_get_sync(dmic->dev);

> > Since we're only allowing reclocking while idle shouldn't the clock
> > already be disabled?  Seems like it ought to be good for power if
> > nothing else...

> We enable the clocks at dai_startup for the DMIC (and disable them on 
> dai_shutdown). We can not reparent while the clocks are enabled.
> This is the reason for this part.

That sounds like the enable is happening too early, then.

> > > +static int omap_dmic_set_clkdiv(struct snd_soc_dai *dai,
> > > +				int div_id, int div)
> > > +{

> > DMIC clocking is usually fairly simple so it seems surprising that the
> > driver isn't able to figure this out for itself.

> The clock towards the external digital mics are based on the DMIC_FCLK rate.
> In case of DMIC_FCLK = 19.2MHz, 24.576MHz we can select between two dividers 
> which will result different clocks for the external microphones.
> I would rather leave this decision to the machine driver which knows the 
> external components, and can pick the best divider for them.

If that's what you're doing then it seems like the machine drivers
should be use set_sysclk() (or perhaps even the clk API) to set up the
rate they're looking for from the visible clock rather than fiddling
about with magic divider values.  That way they can say exactly what
they want directly in terms of the result they're looking for.
--
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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Re: Re: [PATCH 2/5] ASoC: OMAP4: omap-dmic: Initial support for OMAP DMIC
  2011-11-23 10:58       ` Mark Brown
@ 2011-11-23 14:00         ` Péter Ujfalusi
  2011-11-23 14:30           ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: Péter Ujfalusi @ 2011-11-23 14:00 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Tony Lindgren, alsa-devel, linux-omap, Benoit Cousson

On Wednesday 23 November 2011 10:58:07 Mark Brown wrote:
> This just seems like it's making the code needlessly complex - there's
> no harm in holding the reference if you don't enable/disable the clock
> and it makes this function much simpler.

OK.

> > We enable the clocks at dai_startup for the DMIC (and disable them on
> > dai_shutdown). We can not reparent while the clocks are enabled.
> > This is the reason for this part.
> 
> That sounds like the enable is happening too early, then.

This only enables the clock for the DMIC block, the clock for the external 
DMIC will start at trigger time (and stop as well).
In order to access to DMIC registers we need clocks, and the clocks are 
enabled for the duration we have capture stream open.
I would think this is acceptable.

> If that's what you're doing then it seems like the machine drivers
> should be use set_sysclk() (or perhaps even the clk API) to set up the
> rate they're looking for from the visible clock rather than fiddling
> about with magic divider values.  That way they can say exactly what
> they want directly in terms of the result they're looking for.

In OMAP4 DMIC the divider can not be chosen freely.
The clock provided to the external microphones will depend on the selected 
DMIC_FCLK, and the divider.
If I ask the machine driver to ask for specific speed for the external mic, 
the writer of the machine driver anyways have to look up the table from the 
TRM for the possible frequencies. So instead of providing magic divider it has 
to provide magic speed.
I can do that if you prefer that way, but it just going to 'complicate' the 
driver a bit (well not that much, we just will have different way of looking 
up the register value for the divider, and it will be done in 
omap_dmic_set_dai_sysclk instead of omap_dmic_set_clkdiv).

--
Péter
--
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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Re: Re: [PATCH 2/5] ASoC: OMAP4: omap-dmic: Initial support for OMAP DMIC
  2011-11-23 14:00         ` Péter Ujfalusi
@ 2011-11-23 14:30           ` Mark Brown
  2011-11-23 15:24             ` Péter Ujfalusi
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2011-11-23 14:30 UTC (permalink / raw)
  To: Péter Ujfalusi
  Cc: Liam Girdwood, Tony Lindgren, alsa-devel, linux-omap, Benoit Cousson

On Wed, Nov 23, 2011 at 04:00:23PM +0200, Péter Ujfalusi wrote:
> On Wednesday 23 November 2011 10:58:07 Mark Brown wrote:

> > > We enable the clocks at dai_startup for the DMIC (and disable them on
> > > dai_shutdown). We can not reparent while the clocks are enabled.
> > > This is the reason for this part.

> > That sounds like the enable is happening too early, then.

> This only enables the clock for the DMIC block, the clock for the external 
> DMIC will start at trigger time (and stop as well).
> In order to access to DMIC registers we need clocks, and the clocks are 
> enabled for the duration we have capture stream open.
> I would think this is acceptable.

Meh, I guess.  It's hard to love code-wise.

> > If that's what you're doing then it seems like the machine drivers
> > should be use set_sysclk() (or perhaps even the clk API) to set up the
> > rate they're looking for from the visible clock rather than fiddling
> > about with magic divider values.  That way they can say exactly what
> > they want directly in terms of the result they're looking for.

> In OMAP4 DMIC the divider can not be chosen freely.
> The clock provided to the external microphones will depend on the selected 
> DMIC_FCLK, and the divider.
> If I ask the machine driver to ask for specific speed for the external mic, 
> the writer of the machine driver anyways have to look up the table from the 
> TRM for the possible frequencies. So instead of providing magic divider it has 
> to provide magic speed.

Sure, but on the other hand it means that someone reading the machine
driver can tell what's going on without going back to the magic table
either.  Having the rates in the code makes the code more legible and
means that people can at least refer to the DMIC driver for a list of
supported rates rather than having to find the TRM.

I'd also guess that it's much more likely that people will remember
clock rates they can set than divider tables but perhaps that's just me.
--
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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Re: Re: Re: [PATCH 2/5] ASoC: OMAP4: omap-dmic: Initial support for OMAP DMIC
  2011-11-23 14:30           ` Mark Brown
@ 2011-11-23 15:24             ` Péter Ujfalusi
  2011-11-23 15:28               ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: Péter Ujfalusi @ 2011-11-23 15:24 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Tony Lindgren, alsa-devel, linux-omap, Benoit Cousson

On Wednesday 23 November 2011 14:30:50 Mark Brown wrote:
> Meh, I guess.  It's hard to love code-wise.

So you would prefer me to enable the OMAP DMIC's clocks at pcm_trigger:start 
time, and disable them on pcm_trigger:stop?
I have seen cases when the driver did not received the pcm_trigger:stop prior 
to pcm_close call, so in that case a safety check in dai_shutdown is needed to 
be sure the dmic clocks are really disabled.
I would still prefer to keep the dmic clocks enabled for the duration the 
stream is open. On the other hand if I had it in pcm_trigger, I don't need to 
play with the clocks when reparenting...

> > > If that's what you're doing then it seems like the machine drivers
> > > should be use set_sysclk() (or perhaps even the clk API) to set up
> > > the
> > > rate they're looking for from the visible clock rather than fiddling
> > > about with magic divider values.  That way they can say exactly what
> > > they want directly in terms of the result they're looking for.
> > 
> > In OMAP4 DMIC the divider can not be chosen freely.
> > The clock provided to the external microphones will depend on the
> > selected DMIC_FCLK, and the divider.
> > If I ask the machine driver to ask for specific speed for the external
> > mic, the writer of the machine driver anyways have to look up the table
> > from the TRM for the possible frequencies. So instead of providing
> > magic divider it has to provide magic speed.
> 
> Sure, but on the other hand it means that someone reading the machine
> driver can tell what's going on without going back to the magic table
> either.  Having the rates in the code makes the code more legible and
> means that people can at least refer to the DMIC driver for a list of
> supported rates rather than having to find the TRM.
> 
> I'd also guess that it's much more likely that people will remember
> clock rates they can set than divider tables but perhaps that's just me.

Sure, I will change the driver accordingly.

--
Péter
--
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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Re: Re: Re: [PATCH 2/5] ASoC: OMAP4: omap-dmic: Initial support for OMAP DMIC
  2011-11-23 15:24             ` Péter Ujfalusi
@ 2011-11-23 15:28               ` Mark Brown
  0 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2011-11-23 15:28 UTC (permalink / raw)
  To: Péter Ujfalusi
  Cc: Liam Girdwood, Tony Lindgren, alsa-devel, linux-omap, Benoit Cousson

On Wed, Nov 23, 2011 at 05:24:41PM +0200, Péter Ujfalusi wrote:
> On Wednesday 23 November 2011 14:30:50 Mark Brown wrote:
> > Meh, I guess.  It's hard to love code-wise.

> So you would prefer me to enable the OMAP DMIC's clocks at pcm_trigger:start 
> time, and disable them on pcm_trigger:stop?

I don't know that that's the best place, but it does feel like we ought
to be able to do better than we are (and obviously if the clocks are
required for writing to the registers that does change things a little).
Perhaps there's not actually anything better given the restrictions,
it's just that like I say it's hard to love the code.
--
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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2011-11-23 15:28 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-22 14:01 [PATCH 0/5] Support for OMAP4 Digital Microphone interface Peter Ujfalusi
2011-11-22 14:01 ` [PATCH 1/5] OMAP4: hwmod: Add names for DMIC memory address space Peter Ujfalusi
2011-11-22 14:01 ` [PATCH 2/5] ASoC: OMAP4: omap-dmic: Initial support for OMAP DMIC Peter Ujfalusi
2011-11-22 16:01   ` Mark Brown
2011-11-23  8:48     ` Péter Ujfalusi
2011-11-23 10:58       ` Mark Brown
2011-11-23 14:00         ` Péter Ujfalusi
2011-11-23 14:30           ` Mark Brown
2011-11-23 15:24             ` Péter Ujfalusi
2011-11-23 15:28               ` Mark Brown
2011-11-22 14:01 ` [PATCH 3/5] OMAP4: devices: Register OMAP4 DMIC platform device Peter Ujfalusi
2011-11-22 14:01 ` [PATCH 4/5] OMAP4: board-4430sdp: Register platform device for digimic codec Peter Ujfalusi
2011-11-22 14:02 ` [PATCH 5/5] ASoC: sdp4430: Add support for digital microphones Peter Ujfalusi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.