[PATCHv2 6/6] ASoC: OMAP4: Add McPDM platform driver

[PATCHv2 6/6] ASoC: OMAP4: Add McPDM platform driver

[Candelaria Villareal, Jorge]

From: Misael Lopez Cruz <x0052729@ti.com>

McPDM platform driver is configured to use sDMA in order to transfer
to/from memory. Support for interfacing with ABE will be added later.

McPDM dai currently supports up to 4 downlink channels and 2 uplink
channels simultaneously, as well as 88.2 and 96 KHz, and a sample
size of 32 bits.

Signed-off-by: Misael Lopez Cruz <x0052729@ti.com>
Signed-off-by: Margarita Olaya <x0080101@ti.com>
Signed-off-by: Jorge Eduardo Candelaria <x0107209@ti.com>
---
 sound/soc/omap/Makefile     |    2 +-
 sound/soc/omap/omap-mcpdm.c |  250 +++++++++++++++++++++++++++++++++++++++++++
 sound/soc/omap/omap-mcpdm.h |   29 +++++
 3 files changed, 280 insertions(+), 1 deletions(-)
 create mode 100644 sound/soc/omap/omap-mcpdm.c
 create mode 100644 sound/soc/omap/omap-mcpdm.h

diff --git a/sound/soc/omap/Makefile b/sound/soc/omap/Makefile
index bf8b388..d6382fa 100644
--- a/sound/soc/omap/Makefile
+++ b/sound/soc/omap/Makefile
@@ -1,7 +1,7 @@
 # OMAP Platform Support
 snd-soc-omap-objs := omap-pcm.o
 snd-soc-omap-mcbsp-objs := omap-mcbsp.o
-snd-soc-omap-mcpdm-objs := mcpdm.o
+snd-soc-omap-mcpdm-objs := mcpdm.o omap-mcpdm.o
 
 obj-$(CONFIG_SND_OMAP_SOC) += snd-soc-omap.o
 obj-$(CONFIG_SND_OMAP_SOC_MCBSP) += snd-soc-omap-mcbsp.o
diff --git a/sound/soc/omap/omap-mcpdm.c b/sound/soc/omap/omap-mcpdm.c
new file mode 100644
index 0000000..180cca6
--- /dev/null
+++ b/sound/soc/omap/omap-mcpdm.c
@@ -0,0 +1,250 @@
+/*
+ * omap-mcpdm.c  --  OMAP ALSA SoC DAI driver using McPDM port
+ *
+ * Copyright (C) 2009 Texas Instruments
+ *
+ * Author: Misael Lopez Cruz <x0052729@ti.com>
+ * Contact: Jorge Eduardo Candelaria <x0107209@ti.com>
+ *          Margarita Olaya <magi.olaya@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/initval.h>
+#include <sound/soc.h>
+
+#include <mach/control.h>
+#include <mach/dma.h>
+#include <mach/mcbsp.h>
+#include "mcpdm.h"
+#include "omap-mcpdm.h"
+#include "omap-pcm.h"
+
+struct omap_mcpdm_data {
+	struct omap_mcpdm_link *links;
+	int active;
+};
+
+static struct omap_mcpdm_link omap_mcpdm_links[] = {
+	/* downlink */
+	{
+		.irq_mask = MCPDM_DN_IRQ_EMPTY | MCPDM_DN_IRQ_FULL,
+		.threshold = 1,
+		.format = PDMOUTFORMAT_LJUST,
+	},
+	/* uplink */
+	{
+		.irq_mask = MCPDM_UP_IRQ_EMPTY | MCPDM_UP_IRQ_FULL,
+		.threshold = 1,
+		.format = PDMOUTFORMAT_LJUST,
+	},
+};
+
+static struct omap_mcpdm_data mcpdm_data = {
+	.links = omap_mcpdm_links,
+	.active = 0,
+};
+
+/*
+ * Stream DMA parameters
+ */
+static struct omap_pcm_dma_data omap_mcpdm_dai_dma_params[] = {
+	{
+		.name = "Audio downlink",
+		.dma_req = OMAP44XX_DMA_MCPDM_DL,
+		.data_type = OMAP_DMA_DATA_TYPE_S32,
+		.sync_mode = OMAP_DMA_SYNC_PACKET,
+		.packet_size = 16,
+		.port_addr = OMAP44XX_MCPDM_L3_BASE + MCPDM_DN_DATA,
+	},
+	{
+		.name = "Audio uplink",
+		.dma_req = OMAP44XX_DMA_MCPDM_UP,
+		.data_type = OMAP_DMA_DATA_TYPE_S32,
+		.sync_mode = OMAP_DMA_SYNC_PACKET,
+		.packet_size = 16,
+		.port_addr = OMAP44XX_MCPDM_L3_BASE + MCPDM_UP_DATA,
+	},
+};
+
+static int omap_mcpdm_dai_startup(struct snd_pcm_substream *substream,
+				  struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
+	int err = 0;
+
+	if (!cpu_dai->active)
+		err = omap_mcpdm_request();
+
+	return err;
+}
+
+static void omap_mcpdm_dai_shutdown(struct snd_pcm_substream *substream,
+				    struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
+
+	if (!cpu_dai->active)
+		omap_mcpdm_free();
+}
+
+static int omap_mcpdm_dai_trigger(struct snd_pcm_substream *substream, int cmd,
+				  struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
+	struct omap_mcpdm_data *mcpdm_priv = cpu_dai->private_data;
+	int stream = substream->stream;
+	int err = 0;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		if (!mcpdm_priv->active++)
+			omap_mcpdm_start(stream);
+		break;
+
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		if (!--mcpdm_priv->active)
+			omap_mcpdm_stop(stream);
+		break;
+	default:
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+static int omap_mcpdm_dai_hw_params(struct snd_pcm_substream *substream,
+				    struct snd_pcm_hw_params *params,
+				    struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
+	struct omap_mcpdm_data *mcpdm_priv = cpu_dai->private_data;
+	struct omap_mcpdm_link *mcpdm_links = mcpdm_priv->links;
+	int stream = substream->stream;
+	int channels, err, link_mask = 0;
+
+	cpu_dai->dma_data = &omap_mcpdm_dai_dma_params[stream];
+
+	channels = params_channels(params);
+	switch (channels) {
+	case 4:
+		if (stream)
+			/* up to 2 channels for uplink */
+			return -EINVAL;
+		link_mask |= 1 << 3;
+	case 3:
+		if (stream)
+			/* up to 2 channels for uplink */
+			return -EINVAL;
+		link_mask |= 1 << 2;
+	case 2:
+		link_mask |= 1 << 1;
+	case 1:
+		link_mask |= 1 << 0;
+		break;
+	default:
+		/* unsupported number of channels */
+		return -EINVAL;
+	}
+
+	if (stream) {
+		mcpdm_links[stream].channels = link_mask << 0;
+		err = omap_mcpdm_set_uplink(&mcpdm_links[stream]);
+	} else {
+		mcpdm_links[stream].channels = link_mask << 3;
+		err = omap_mcpdm_set_downlink(&mcpdm_links[stream]);
+	}
+
+	return err;
+}
+
+static int omap_mcpdm_dai_hw_free(struct snd_pcm_substream *substream,
+				  struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
+	struct omap_mcpdm_data *mcpdm_priv = cpu_dai->private_data;
+	struct omap_mcpdm_link *mcpdm_links = mcpdm_priv->links;
+	int err;
+
+	if (substream->stream)
+		err = omap_mcpdm_clr_uplink(&mcpdm_links[substream->stream]);
+	else
+		err = omap_mcpdm_clr_downlink(&mcpdm_links[substream->stream]);
+
+	return err;
+}
+
+static struct snd_soc_dai_ops omap_mcpdm_dai_ops = {
+	.startup	= omap_mcpdm_dai_startup,
+	.shutdown	= omap_mcpdm_dai_shutdown,
+	.trigger	= omap_mcpdm_dai_trigger,
+	.hw_params	= omap_mcpdm_dai_hw_params,
+	.hw_free	= omap_mcpdm_dai_hw_free,
+};
+
+#define OMAP_MCPDM_RATES	(SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000)
+#define OMAP_MCPDM_FORMATS	(SNDRV_PCM_FMTBIT_S32_LE)
+
+struct snd_soc_dai omap_mcpdm_dai = {
+	.name = "omap-mcpdm",
+	.id = -1,
+	.playback = {
+		.channels_min = 1,
+		.channels_max = 4,
+		.rates = OMAP_MCPDM_RATES,
+		.formats = OMAP_MCPDM_FORMATS,
+	},
+	.capture = {
+		.channels_min = 1,
+		.channels_max = 2,
+		.rates = OMAP_MCPDM_RATES,
+		.formats = OMAP_MCPDM_FORMATS,
+	},
+	.ops = &omap_mcpdm_dai_ops,
+	.private_data = &mcpdm_data,
+};
+EXPORT_SYMBOL_GPL(omap_mcpdm_dai);
+
+static int __init snd_omap_mcpdm_init(void)
+{
+	return snd_soc_register_dai(&omap_mcpdm_dai);
+}
+module_init(snd_omap_mcpdm_init);
+
+static void __exit snd_omap_mcpdm_exit(void)
+{
+	snd_soc_unregister_dai(&omap_mcpdm_dai);
+}
+module_exit(snd_omap_mcpdm_exit);
+
+MODULE_AUTHOR("Misael Lopez Cruz <x0052729@ti.com>");
+MODULE_DESCRIPTION("OMAP PDM SoC Interface");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/omap/omap-mcpdm.h b/sound/soc/omap/omap-mcpdm.h
new file mode 100644
index 0000000..73b80d5
--- /dev/null
+++ b/sound/soc/omap/omap-mcpdm.h
@@ -0,0 +1,29 @@
+/*
+ * omap-mcpdm.h
+ *
+ * Copyright (C) 2009 Texas Instruments
+ *
+ * Contact: Misael Lopez Cruz <x0052729@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#ifndef __OMAP_MCPDM_H__
+#define __OMAP_MCPDM_H__
+
+extern struct snd_soc_dai omap_mcpdm_dai;
+
+#endif	/* End of __OMAP_MCPDM_H__ */
-- 
1.6.0.4

Re: [PATCHv2 6/6] ASoC: OMAP4: Add McPDM platform driver

[Liam Girdwood]

On Fri, 2010-01-22 at 17:15 -0600, Candelaria Villareal, Jorge wrote:
> From: Misael Lopez Cruz <x0052729@ti.com>
> 
> McPDM platform driver is configured to use sDMA in order to transfer
> to/from memory. Support for interfacing with ABE will be added later.
> 
> McPDM dai currently supports up to 4 downlink channels and 2 uplink
> channels simultaneously, as well as 88.2 and 96 KHz, and a sample
> size of 32 bits.
> 
> Signed-off-by: Misael Lopez Cruz <x0052729@ti.com>
> Signed-off-by: Margarita Olaya <x0080101@ti.com>
> Signed-off-by: Jorge Eduardo Candelaria <x0107209@ti.com>
> ---
>  sound/soc/omap/Makefile     |    2 +-
>  sound/soc/omap/omap-mcpdm.c |  250 +++++++++++++++++++++++++++++++++++++++++++
>  sound/soc/omap/omap-mcpdm.h |   29 +++++
>  3 files changed, 280 insertions(+), 1 deletions(-)
>  create mode 100644 sound/soc/omap/omap-mcpdm.c
>  create mode 100644 sound/soc/omap/omap-mcpdm.h
> 
> diff --git a/sound/soc/omap/Makefile b/sound/soc/omap/Makefile
> index bf8b388..d6382fa 100644
> --- a/sound/soc/omap/Makefile
> +++ b/sound/soc/omap/Makefile
> @@ -1,7 +1,7 @@
>  # OMAP Platform Support
>  snd-soc-omap-objs := omap-pcm.o
>  snd-soc-omap-mcbsp-objs := omap-mcbsp.o
> -snd-soc-omap-mcpdm-objs := mcpdm.o
> +snd-soc-omap-mcpdm-objs := mcpdm.o omap-mcpdm.o
>  
>  obj-$(CONFIG_SND_OMAP_SOC) += snd-soc-omap.o
>  obj-$(CONFIG_SND_OMAP_SOC_MCBSP) += snd-soc-omap-mcbsp.o
> diff --git a/sound/soc/omap/omap-mcpdm.c b/sound/soc/omap/omap-mcpdm.c
> new file mode 100644
> index 0000000..180cca6
> --- /dev/null
> +++ b/sound/soc/omap/omap-mcpdm.c
> @@ -0,0 +1,250 @@
> +/*
> + * omap-mcpdm.c  --  OMAP ALSA SoC DAI driver using McPDM port
> + *
> + * Copyright (C) 2009 Texas Instruments
> + *
> + * Author: Misael Lopez Cruz <x0052729@ti.com>
> + * Contact: Jorge Eduardo Candelaria <x0107209@ti.com>
> + *          Margarita Olaya <magi.olaya@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110-1301 USA
> + *
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <sound/core.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/initval.h>
> +#include <sound/soc.h>
> +
> +#include <mach/control.h>
> +#include <mach/dma.h>
> +#include <mach/mcbsp.h>
> +#include "mcpdm.h"
> +#include "omap-mcpdm.h"
> +#include "omap-pcm.h"
> +
> +struct omap_mcpdm_data {
> +	struct omap_mcpdm_link *links;
> +	int active;
> +};
> +
> +static struct omap_mcpdm_link omap_mcpdm_links[] = {
> +	/* downlink */
> +	{
> +		.irq_mask = MCPDM_DN_IRQ_EMPTY | MCPDM_DN_IRQ_FULL,
> +		.threshold = 1,
> +		.format = PDMOUTFORMAT_LJUST,
> +	},
> +	/* uplink */
> +	{
> +		.irq_mask = MCPDM_UP_IRQ_EMPTY | MCPDM_UP_IRQ_FULL,
> +		.threshold = 1,
> +		.format = PDMOUTFORMAT_LJUST,
> +	},
> +};
> +
> +static struct omap_mcpdm_data mcpdm_data = {
> +	.links = omap_mcpdm_links,
> +	.active = 0,
> +};
> +
> +/*
> + * Stream DMA parameters
> + */
> +static struct omap_pcm_dma_data omap_mcpdm_dai_dma_params[] = {
> +	{
> +		.name = "Audio downlink",
> +		.dma_req = OMAP44XX_DMA_MCPDM_DL,
> +		.data_type = OMAP_DMA_DATA_TYPE_S32,
> +		.sync_mode = OMAP_DMA_SYNC_PACKET,
> +		.packet_size = 16,
> +		.port_addr = OMAP44XX_MCPDM_L3_BASE + MCPDM_DN_DATA,
> +	},
> +	{
> +		.name = "Audio uplink",
> +		.dma_req = OMAP44XX_DMA_MCPDM_UP,
> +		.data_type = OMAP_DMA_DATA_TYPE_S32,
> +		.sync_mode = OMAP_DMA_SYNC_PACKET,
> +		.packet_size = 16,
> +		.port_addr = OMAP44XX_MCPDM_L3_BASE + MCPDM_UP_DATA,
> +	},
> +};
> +
> +static int omap_mcpdm_dai_startup(struct snd_pcm_substream *substream,
> +				  struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> +	int err = 0;
> +
> +	if (!cpu_dai->active)
> +		err = omap_mcpdm_request();

Will anything else use this hw interface other than ALSA audio ?
If not, the request is probably better in the machine driver probe().

> +
> +	return err;
> +}
> +
> +static void omap_mcpdm_dai_shutdown(struct snd_pcm_substream *substream,
> +				    struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> +
> +	if (!cpu_dai->active)
> +		omap_mcpdm_free();

ditto.

> +}
> +
> +static int omap_mcpdm_dai_trigger(struct snd_pcm_substream *substream, int cmd,
> +				  struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> +	struct omap_mcpdm_data *mcpdm_priv = cpu_dai->private_data;
> +	int stream = substream->stream;
> +	int err = 0;
> +
> +	switch (cmd) {
> +	case SNDRV_PCM_TRIGGER_START:
> +	case SNDRV_PCM_TRIGGER_RESUME:
> +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> +		if (!mcpdm_priv->active++)
> +			omap_mcpdm_start(stream);
> +		break;
> +
> +	case SNDRV_PCM_TRIGGER_STOP:
> +	case SNDRV_PCM_TRIGGER_SUSPEND:
> +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> +		if (!--mcpdm_priv->active)
> +			omap_mcpdm_stop(stream);
> +		break;
> +	default:
> +		err = -EINVAL;
> +	}
> +
> +	return err;
> +}
> +
> +static int omap_mcpdm_dai_hw_params(struct snd_pcm_substream *substream,
> +				    struct snd_pcm_hw_params *params,
> +				    struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> +	struct omap_mcpdm_data *mcpdm_priv = cpu_dai->private_data;
> +	struct omap_mcpdm_link *mcpdm_links = mcpdm_priv->links;
> +	int stream = substream->stream;
> +	int channels, err, link_mask = 0;
> +
> +	cpu_dai->dma_data = &omap_mcpdm_dai_dma_params[stream];
> +
> +	channels = params_channels(params);
> +	switch (channels) {
> +	case 4:
> +		if (stream)

stream is either PLAYBACK or CAPTURE here.

> +			/* up to 2 channels for uplink */
> +			return -EINVAL;
> +		link_mask |= 1 << 3;
> +	case 3:
> +		if (stream)
> +			/* up to 2 channels for uplink */
> +			return -EINVAL;
> +		link_mask |= 1 << 2;
> +	case 2:
> +		link_mask |= 1 << 1;
> +	case 1:
> +		link_mask |= 1 << 0;
> +		break;
> +	default:
> +		/* unsupported number of channels */
> +		return -EINVAL;
> +	}
> +
> +	if (stream) {

ditto

> +		mcpdm_links[stream].channels = link_mask << 0;
> +		err = omap_mcpdm_set_uplink(&mcpdm_links[stream]);
> +	} else {
> +		mcpdm_links[stream].channels = link_mask << 3;
> +		err = omap_mcpdm_set_downlink(&mcpdm_links[stream]);
> +	}
> +
> +	return err;
> +}
> +
> +static int omap_mcpdm_dai_hw_free(struct snd_pcm_substream *substream,
> +				  struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> +	struct omap_mcpdm_data *mcpdm_priv = cpu_dai->private_data;
> +	struct omap_mcpdm_link *mcpdm_links = mcpdm_priv->links;
> +	int err;
> +
> +	if (substream->stream)


ditto

> +		err = omap_mcpdm_clr_uplink(&mcpdm_links[substream->stream]);
> +	else
> +		err = omap_mcpdm_clr_downlink(&mcpdm_links[substream->stream]);
> +
> +	return err;
> +}
> +
> +static struct snd_soc_dai_ops omap_mcpdm_dai_ops = {
> +	.startup	= omap_mcpdm_dai_startup,
> +	.shutdown	= omap_mcpdm_dai_shutdown,
> +	.trigger	= omap_mcpdm_dai_trigger,
> +	.hw_params	= omap_mcpdm_dai_hw_params,
> +	.hw_free	= omap_mcpdm_dai_hw_free,
> +};
> +
> +#define OMAP_MCPDM_RATES	(SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000)

Is this the link rate or sample rate ? What about slower rates ?

> +#define OMAP_MCPDM_FORMATS	(SNDRV_PCM_FMTBIT_S32_LE)
> +

Is this the link format or sample format ? What about 16bit ?

> +struct snd_soc_dai omap_mcpdm_dai = {
> +	.name = "omap-mcpdm",
> +	.id = -1,
> +	.playback = {
> +		.channels_min = 1,
> +		.channels_max = 4,
> +		.rates = OMAP_MCPDM_RATES,
> +		.formats = OMAP_MCPDM_FORMATS,
> +	},
> +	.capture = {
> +		.channels_min = 1,
> +		.channels_max = 2,
> +		.rates = OMAP_MCPDM_RATES,
> +		.formats = OMAP_MCPDM_FORMATS,
> +	},
> +	.ops = &omap_mcpdm_dai_ops,
> +	.private_data = &mcpdm_data,
> +};
> +EXPORT_SYMBOL_GPL(omap_mcpdm_dai);
> +
> +static int __init snd_omap_mcpdm_init(void)
> +{
> +	return snd_soc_register_dai(&omap_mcpdm_dai);
> +}
> +module_init(snd_omap_mcpdm_init);
> +
> +static void __exit snd_omap_mcpdm_exit(void)
> +{
> +	snd_soc_unregister_dai(&omap_mcpdm_dai);
> +}
> +module_exit(snd_omap_mcpdm_exit);
> +
> +MODULE_AUTHOR("Misael Lopez Cruz <x0052729@ti.com>");
> +MODULE_DESCRIPTION("OMAP PDM SoC Interface");
> +MODULE_LICENSE("GPL");
> diff --git a/sound/soc/omap/omap-mcpdm.h b/sound/soc/omap/omap-mcpdm.h
> new file mode 100644
> index 0000000..73b80d5
> --- /dev/null
> +++ b/sound/soc/omap/omap-mcpdm.h
> @@ -0,0 +1,29 @@
> +/*
> + * omap-mcpdm.h
> + *
> + * Copyright (C) 2009 Texas Instruments
> + *
> + * Contact: Misael Lopez Cruz <x0052729@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> + * 02110-1301 USA
> + *
> + */
> +
> +#ifndef __OMAP_MCPDM_H__
> +#define __OMAP_MCPDM_H__
> +
> +extern struct snd_soc_dai omap_mcpdm_dai;
> +
> +#endif	/* End of __OMAP_MCPDM_H__ */

RE: [alsa-devel] [PATCHv2 6/6] ASoC: OMAP4: Add McPDM platform driver

[Candelaria Villareal, Jorge]

From: Liam Girdwood wrote:
> On Fri, 2010-01-22 at 17:15 -0600, Candelaria Villareal, Jorge wrote:
> > From: Misael Lopez Cruz <x0052729@ti.com>
> > 
> > McPDM platform driver is configured to use sDMA in order to transfer
> > to/from memory. Support for interfacing with ABE will be 
> added later.
> > 
> > McPDM dai currently supports up to 4 downlink channels and 2 uplink
> > channels simultaneously, as well as 88.2 and 96 KHz, and a sample
> > size of 32 bits.
> > 
> > Signed-off-by: Misael Lopez Cruz <x0052729@ti.com>
> > Signed-off-by: Margarita Olaya <x0080101@ti.com>
> > Signed-off-by: Jorge Eduardo Candelaria <x0107209@ti.com>
> > ---
> >  sound/soc/omap/Makefile     |    2 +-
> >  sound/soc/omap/omap-mcpdm.c |  250 
> +++++++++++++++++++++++++++++++++++++++++++
> >  sound/soc/omap/omap-mcpdm.h |   29 +++++
> >  3 files changed, 280 insertions(+), 1 deletions(-)
> >  create mode 100644 sound/soc/omap/omap-mcpdm.c
> >  create mode 100644 sound/soc/omap/omap-mcpdm.h
> > 
> > diff --git a/sound/soc/omap/Makefile b/sound/soc/omap/Makefile
> > index bf8b388..d6382fa 100644
> > --- a/sound/soc/omap/Makefile
> > +++ b/sound/soc/omap/Makefile
> > @@ -1,7 +1,7 @@
> >  # OMAP Platform Support
> >  snd-soc-omap-objs := omap-pcm.o
> >  snd-soc-omap-mcbsp-objs := omap-mcbsp.o
> > -snd-soc-omap-mcpdm-objs := mcpdm.o
> > +snd-soc-omap-mcpdm-objs := mcpdm.o omap-mcpdm.o
> >  
> >  obj-$(CONFIG_SND_OMAP_SOC) += snd-soc-omap.o
> >  obj-$(CONFIG_SND_OMAP_SOC_MCBSP) += snd-soc-omap-mcbsp.o
> > diff --git a/sound/soc/omap/omap-mcpdm.c 
> b/sound/soc/omap/omap-mcpdm.c
> > new file mode 100644
> > index 0000000..180cca6
> > --- /dev/null
> > +++ b/sound/soc/omap/omap-mcpdm.c
> > @@ -0,0 +1,250 @@
> > +/*
> > + * omap-mcpdm.c  --  OMAP ALSA SoC DAI driver using McPDM port
> > + *
> > + * Copyright (C) 2009 Texas Instruments
> > + *
> > + * Author: Misael Lopez Cruz <x0052729@ti.com>
> > + * Contact: Jorge Eduardo Candelaria <x0107209@ti.com>
> > + *          Margarita Olaya <magi.olaya@ti.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be 
> useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  
> See the GNU
> > + * General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General 
> Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> > + * 02110-1301 USA
> > + *
> > + */
> > +
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/device.h>
> > +#include <sound/core.h>
> > +#include <sound/pcm.h>
> > +#include <sound/pcm_params.h>
> > +#include <sound/initval.h>
> > +#include <sound/soc.h>
> > +
> > +#include <mach/control.h>
> > +#include <mach/dma.h>
> > +#include <mach/mcbsp.h>
> > +#include "mcpdm.h"
> > +#include "omap-mcpdm.h"
> > +#include "omap-pcm.h"
> > +
> > +struct omap_mcpdm_data {
> > +	struct omap_mcpdm_link *links;
> > +	int active;
> > +};
> > +
> > +static struct omap_mcpdm_link omap_mcpdm_links[] = {
> > +	/* downlink */
> > +	{
> > +		.irq_mask = MCPDM_DN_IRQ_EMPTY | MCPDM_DN_IRQ_FULL,
> > +		.threshold = 1,
> > +		.format = PDMOUTFORMAT_LJUST,
> > +	},
> > +	/* uplink */
> > +	{
> > +		.irq_mask = MCPDM_UP_IRQ_EMPTY | MCPDM_UP_IRQ_FULL,
> > +		.threshold = 1,
> > +		.format = PDMOUTFORMAT_LJUST,
> > +	},
> > +};
> > +
> > +static struct omap_mcpdm_data mcpdm_data = {
> > +	.links = omap_mcpdm_links,
> > +	.active = 0,
> > +};
> > +
> > +/*
> > + * Stream DMA parameters
> > + */
> > +static struct omap_pcm_dma_data omap_mcpdm_dai_dma_params[] = {
> > +	{
> > +		.name = "Audio downlink",
> > +		.dma_req = OMAP44XX_DMA_MCPDM_DL,
> > +		.data_type = OMAP_DMA_DATA_TYPE_S32,
> > +		.sync_mode = OMAP_DMA_SYNC_PACKET,
> > +		.packet_size = 16,
> > +		.port_addr = OMAP44XX_MCPDM_L3_BASE + MCPDM_DN_DATA,
> > +	},
> > +	{
> > +		.name = "Audio uplink",
> > +		.dma_req = OMAP44XX_DMA_MCPDM_UP,
> > +		.data_type = OMAP_DMA_DATA_TYPE_S32,
> > +		.sync_mode = OMAP_DMA_SYNC_PACKET,
> > +		.packet_size = 16,
> > +		.port_addr = OMAP44XX_MCPDM_L3_BASE + MCPDM_UP_DATA,
> > +	},
> > +};
> > +
> > +static int omap_mcpdm_dai_startup(struct snd_pcm_substream 
> *substream,
> > +				  struct snd_soc_dai *dai)
> > +{
> > +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> > +	int err = 0;
> > +
> > +	if (!cpu_dai->active)
> > +		err = omap_mcpdm_request();
> 
> Will anything else use this hw interface other than ALSA audio ?
> If not, the request is probably better in the machine driver probe().

omap_mcpdm_request will enable the functional clock. Isn't it better
for the clock to be enabled only when is about to get used?

> 
> > +
> > +	return err;
> > +}
> > +
> > +static void omap_mcpdm_dai_shutdown(struct 
> snd_pcm_substream *substream,
> > +				    struct snd_soc_dai *dai)
> > +{
> > +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> > +
> > +	if (!cpu_dai->active)
> > +		omap_mcpdm_free();
> 
> ditto.

Also, omap_mcpdm_free should disable the clock once it is not needed.

> 
> > +}
> > +
> > +static int omap_mcpdm_dai_trigger(struct snd_pcm_substream 
> *substream, int cmd,
> > +				  struct snd_soc_dai *dai)
> > +{
> > +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> > +	struct omap_mcpdm_data *mcpdm_priv = cpu_dai->private_data;
> > +	int stream = substream->stream;
> > +	int err = 0;
> > +
> > +	switch (cmd) {
> > +	case SNDRV_PCM_TRIGGER_START:
> > +	case SNDRV_PCM_TRIGGER_RESUME:
> > +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> > +		if (!mcpdm_priv->active++)
> > +			omap_mcpdm_start(stream);
> > +		break;
> > +
> > +	case SNDRV_PCM_TRIGGER_STOP:
> > +	case SNDRV_PCM_TRIGGER_SUSPEND:
> > +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> > +		if (!--mcpdm_priv->active)
> > +			omap_mcpdm_stop(stream);
> > +		break;
> > +	default:
> > +		err = -EINVAL;
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +static int omap_mcpdm_dai_hw_params(struct 
> snd_pcm_substream *substream,
> > +				    struct snd_pcm_hw_params *params,
> > +				    struct snd_soc_dai *dai)
> > +{
> > +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> > +	struct omap_mcpdm_data *mcpdm_priv = cpu_dai->private_data;
> > +	struct omap_mcpdm_link *mcpdm_links = mcpdm_priv->links;
> > +	int stream = substream->stream;
> > +	int channels, err, link_mask = 0;
> > +
> > +	cpu_dai->dma_data = &omap_mcpdm_dai_dma_params[stream];
> > +
> > +	channels = params_channels(params);
> > +	switch (channels) {
> > +	case 4:
> > +		if (stream)
> 
> stream is either PLAYBACK or CAPTURE here.

Downlink and uplink were chosen instead to avoid confusion
because that is how it is referred in McPDM technical
documentation. But I do understand that we are talking
about ALSA stream, not McPDM...

> 
> > +			/* up to 2 channels for uplink */

So I should refer to CAPTURE instead of UPLINK in here.

> > +			return -EINVAL;
> > +		link_mask |= 1 << 3;
> > +	case 3:
> > +		if (stream)
> > +			/* up to 2 channels for uplink */
> > +			return -EINVAL;
> > +		link_mask |= 1 << 2;
> > +	case 2:
> > +		link_mask |= 1 << 1;
> > +	case 1:
> > +		link_mask |= 1 << 0;
> > +		break;
> > +	default:
> > +		/* unsupported number of channels */
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (stream) {
> 
> ditto

Here, however, the methods set_uplink & set_downlink are
Named after McPDM data paths. In this case is it preferred
to name everything using ALSA convention? Or would it be ok
to use downlink/uplink on certain cases?

> 
> > +		mcpdm_links[stream].channels = link_mask << 0;
> > +		err = omap_mcpdm_set_uplink(&mcpdm_links[stream]);
> > +	} else {
> > +		mcpdm_links[stream].channels = link_mask << 3;
> > +		err = omap_mcpdm_set_downlink(&mcpdm_links[stream]);
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +static int omap_mcpdm_dai_hw_free(struct snd_pcm_substream 
> *substream,
> > +				  struct snd_soc_dai *dai)
> > +{
> > +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> > +	struct omap_mcpdm_data *mcpdm_priv = cpu_dai->private_data;
> > +	struct omap_mcpdm_link *mcpdm_links = mcpdm_priv->links;
> > +	int err;
> > +
> > +	if (substream->stream)
> 
> 
> ditto

This is the same case, uplink and downlink refer to McPDM data
path names.
 
> > +		err = 
> omap_mcpdm_clr_uplink(&mcpdm_links[substream->stream]);
> > +	else
> > +		err = 
> omap_mcpdm_clr_downlink(&mcpdm_links[substream->stream]);
> > +
> > +	return err;
> > +}
> > +
> > +static struct snd_soc_dai_ops omap_mcpdm_dai_ops = {
> > +	.startup	= omap_mcpdm_dai_startup,
> > +	.shutdown	= omap_mcpdm_dai_shutdown,
> > +	.trigger	= omap_mcpdm_dai_trigger,
> > +	.hw_params	= omap_mcpdm_dai_hw_params,
> > +	.hw_free	= omap_mcpdm_dai_hw_free,
> > +};
> > +
> > +#define OMAP_MCPDM_RATES	(SNDRV_PCM_RATE_88200 | 
> SNDRV_PCM_RATE_96000)
> 
> Is this the link rate or sample rate ? What about slower rates ?

This is the sample rate. Slower rates are not supported by McPDM
nor by the audio codec. When ABE is present, it supports other
input rates, like 48000, but then ABE needs to upsample to 96000
before sending to McPDM module.

> 
> > +#define OMAP_MCPDM_FORMATS	(SNDRV_PCM_FMTBIT_S32_LE)
> > +
> 
> Is this the link format or sample format ? What about 16bit ?

Again, only ABE supports 16 bit, but needs to convert to 32 bit
before sending to McPDM. With current McPDM patches, we are
bypassing ABE.

> 
> > +struct snd_soc_dai omap_mcpdm_dai = {
> > +	.name = "omap-mcpdm",
> > +	.id = -1,
> > +	.playback = {
> > +		.channels_min = 1,
> > +		.channels_max = 4,
> > +		.rates = OMAP_MCPDM_RATES,
> > +		.formats = OMAP_MCPDM_FORMATS,
> > +	},
> > +	.capture = {
> > +		.channels_min = 1,
> > +		.channels_max = 2,
> > +		.rates = OMAP_MCPDM_RATES,
> > +		.formats = OMAP_MCPDM_FORMATS,
> > +	},
> > +	.ops = &omap_mcpdm_dai_ops,
> > +	.private_data = &mcpdm_data,
> > +};
> > +EXPORT_SYMBOL_GPL(omap_mcpdm_dai);
> > +
> > +static int __init snd_omap_mcpdm_init(void)
> > +{
> > +	return snd_soc_register_dai(&omap_mcpdm_dai);
> > +}
> > +module_init(snd_omap_mcpdm_init);
> > +
> > +static void __exit snd_omap_mcpdm_exit(void)
> > +{
> > +	snd_soc_unregister_dai(&omap_mcpdm_dai);
> > +}
> > +module_exit(snd_omap_mcpdm_exit);
> > +
> > +MODULE_AUTHOR("Misael Lopez Cruz <x0052729@ti.com>");
> > +MODULE_DESCRIPTION("OMAP PDM SoC Interface");
> > +MODULE_LICENSE("GPL");
> > diff --git a/sound/soc/omap/omap-mcpdm.h 
> b/sound/soc/omap/omap-mcpdm.h
> > new file mode 100644
> > index 0000000..73b80d5
> > --- /dev/null
> > +++ b/sound/soc/omap/omap-mcpdm.h
> > @@ -0,0 +1,29 @@
> > +/*
> > + * omap-mcpdm.h
> > + *
> > + * Copyright (C) 2009 Texas Instruments
> > + *
> > + * Contact: Misael Lopez Cruz <x0052729@ti.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be 
> useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  
> See the GNU
> > + * General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General 
> Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> > + * 02110-1301 USA
> > + *
> > + */
> > +
> > +#ifndef __OMAP_MCPDM_H__
> > +#define __OMAP_MCPDM_H__
> > +
> > +extern struct snd_soc_dai omap_mcpdm_dai;
> > +
> > +#endif	/* End of __OMAP_MCPDM_H__ */
> 
> 
> 

Re: [PATCHv2 6/6] ASoC: OMAP4: Add McPDM platform driver

[Jarkko Nikula]

Hi

On Mon, 25 Jan 2010 15:06:44 -0600
"Candelaria Villareal, Jorge" <jorge.candelaria@ti.com> wrote:

> > > +static int omap_mcpdm_dai_startup(struct snd_pcm_substream 
> > *substream,
> > > +				  struct snd_soc_dai *dai)
> > > +{
> > > +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > > +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> > > +	int err = 0;
> > > +
> > > +	if (!cpu_dai->active)
> > > +		err = omap_mcpdm_request();
> > 
> > Will anything else use this hw interface other than ALSA audio ?
> > If not, the request is probably better in the machine driver probe().
> 
> omap_mcpdm_request will enable the functional clock. Isn't it better
> for the clock to be enabled only when is about to get used?
> 
Definitely yes if there is no any need to keep block active after the
request. That would help the power-management if there are no active
clocks when the streams are suspended with omap_mcpdm_stop() but the
block remains reserved (i.e. omap_mcpdm_free is not called).

The current McBSP implementation is not the best example here but
hopefully that will get fixed at some point.

> > stream is either PLAYBACK or CAPTURE here.
> 
> Downlink and uplink were chosen instead to avoid confusion
> because that is how it is referred in McPDM technical
> documentation. But I do understand that we are talking
> about ALSA stream, not McPDM...
...
> Here, however, the methods set_uplink & set_downlink are
> Named after McPDM data paths. In this case is it preferred
> to name everything using ALSA convention? Or would it be ok
> to use downlink/uplink on certain cases?
> 
I think something like below is clear enough. It is easy to see that we
are dealing with ALSA playback stream which corresponds to downlink path
in HW.

if (stream == SNDRV_PCM_STREAM_PLAYBACK)
	omap_mcpdm_downlink_opxxx();

Also for the user(-space) standard playback and capture naming is more
clear than downlink/uplink terminology.


-- 
Jarkko

Re: [alsa-devel] [PATCHv2 6/6] ASoC: OMAP4: Add McPDM platform driver

[Liam Girdwood]

On Tue, 2010-01-26 at 10:27 +0200, Jarkko Nikula wrote:
> Hi
> 
> On Mon, 25 Jan 2010 15:06:44 -0600
> "Candelaria Villareal, Jorge" <jorge.candelaria@ti.com> wrote:
> 
> > > > +static int omap_mcpdm_dai_startup(struct snd_pcm_substream 
> > > *substream,
> > > > +				  struct snd_soc_dai *dai)
> > > > +{
> > > > +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > > > +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> > > > +	int err = 0;
> > > > +
> > > > +	if (!cpu_dai->active)
> > > > +		err = omap_mcpdm_request();
> > > 
> > > Will anything else use this hw interface other than ALSA audio ?
> > > If not, the request is probably better in the machine driver probe().
> > 
> > omap_mcpdm_request will enable the functional clock. Isn't it better
> > for the clock to be enabled only when is about to get used?
> > 
> Definitely yes if there is no any need to keep block active after the
> request. That would help the power-management if there are no active
> clocks when the streams are suspended with omap_mcpdm_stop() but the
> block remains reserved (i.e. omap_mcpdm_free is not called).


Ah ok, it seems that some other platforms have similar get_resource_x()
functions that do not additionally call their clk_enable_resource_x().
(last time I looked).

This is fine.

Liam 

RE: [alsa-devel] [PATCHv2 6/6] ASoC: OMAP4: Add McPDM platform driver

[Liam Girdwood]

On Mon, 2010-01-25 at 15:06 -0600, Candelaria Villareal, Jorge wrote:
> From: Liam Girdwood wrote:
> > On Fri, 2010-01-22 at 17:15 -0600, Candelaria Villareal, Jorge wrote:

snip

> > > +static int omap_mcpdm_dai_hw_params(struct 
> > snd_pcm_substream *substream,
> > > +				    struct snd_pcm_hw_params *params,
> > > +				    struct snd_soc_dai *dai)
> > > +{
> > > +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > > +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> > > +	struct omap_mcpdm_data *mcpdm_priv = cpu_dai->private_data;
> > > +	struct omap_mcpdm_link *mcpdm_links = mcpdm_priv->links;
> > > +	int stream = substream->stream;
> > > +	int channels, err, link_mask = 0;
> > > +
> > > +	cpu_dai->dma_data = &omap_mcpdm_dai_dma_params[stream];
> > > +
> > > +	channels = params_channels(params);
> > > +	switch (channels) {
> > > +	case 4:
> > > +		if (stream)
> > 
> > stream is either PLAYBACK or CAPTURE here.
> 
> Downlink and uplink were chosen instead to avoid confusion
> because that is how it is referred in McPDM technical
> documentation. But I do understand that we are talking
> about ALSA stream, not McPDM...
> 
> > 
> > > +			/* up to 2 channels for uplink */
> 
> So I should refer to CAPTURE instead of UPLINK in here.

I think it's best to use both CAPTURE and UPLINK in the comments.

> 
> > > +			return -EINVAL;
> > > +		link_mask |= 1 << 3;
> > > +	case 3:
> > > +		if (stream)
> > > +			/* up to 2 channels for uplink */
> > > +			return -EINVAL;
> > > +		link_mask |= 1 << 2;
> > > +	case 2:
> > > +		link_mask |= 1 << 1;
> > > +	case 1:
> > > +		link_mask |= 1 << 0;
> > > +		break;
> > > +	default:
> > > +		/* unsupported number of channels */
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	if (stream) {
> > 
> > ditto
> 
> Here, however, the methods set_uplink & set_downlink are
> Named after McPDM data paths. In this case is it preferred
> to name everything using ALSA convention? Or would it be ok
> to use downlink/uplink on certain cases?

I would name after ALSA but also have comments to state mcpdm link.

Thanks

Liam

RE: [alsa-devel] [PATCHv2 6/6] ASoC: OMAP4: Add McPDM platform driver

[Candelaria Villareal, Jorge]

 

Jarkko Nikula wrote:
> Hi
> 
> On Mon, 25 Jan 2010 15:06:44 -0600
> "Candelaria Villareal, Jorge" <jorge.candelaria@ti.com> wrote:
> 
> > > > +static int omap_mcpdm_dai_startup(struct snd_pcm_substream 
> > > *substream,
> > > > +				  struct snd_soc_dai *dai)
> > > > +{
> > > > +	struct snd_soc_pcm_runtime *rtd = 
> substream->private_data;
> > > > +	struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> > > > +	int err = 0;
> > > > +
> > > > +	if (!cpu_dai->active)
> > > > +		err = omap_mcpdm_request();
> > > 
> > > Will anything else use this hw interface other than ALSA audio ?
> > > If not, the request is probably better in the machine 
> driver probe().
> > 
> > omap_mcpdm_request will enable the functional clock. Isn't it better
> > for the clock to be enabled only when is about to get used?
> > 
> Definitely yes if there is no any need to keep block active after the
> request. That would help the power-management if there are no active
> clocks when the streams are suspended with omap_mcpdm_stop() but the
> block remains reserved (i.e. omap_mcpdm_free is not called).
> 
> The current McBSP implementation is not the best example here but
> hopefully that will get fixed at some point.
> 
> > > stream is either PLAYBACK or CAPTURE here.
> > 
> > Downlink and uplink were chosen instead to avoid confusion
> > because that is how it is referred in McPDM technical
> > documentation. But I do understand that we are talking
> > about ALSA stream, not McPDM...
> ...
> > Here, however, the methods set_uplink & set_downlink are
> > Named after McPDM data paths. In this case is it preferred
> > to name everything using ALSA convention? Or would it be ok
> > to use downlink/uplink on certain cases?
> > 
> I think something like below is clear enough. It is easy to 
> see that we
> are dealing with ALSA playback stream which corresponds to 
> downlink path
> in HW.
> 
> if (stream == SNDRV_PCM_STREAM_PLAYBACK)
> 	omap_mcpdm_downlink_opxxx();
> 
> Also for the user(-space) standard playback and capture naming is more
> clear than downlink/uplink terminology.

So the idea would be to show that ALSA playback/capture correspond
to McPDM downlink/uplink paths. Sounds good, I can try this for all
downlink/uplink operations.

> 
> 
> -- 
> Jarkko
>