[PATCHv4 0/2] McBSP: OMAP3: Add sidetone feature

[PATCHv4 0/2] McBSP: OMAP3: Add sidetone feature

[Ilkka Koskinen]

The first patch implements McBSP sidetone feature on OMAP3 while the second
one provides ALSA interface for using it.

The patch set is based on the patch Eduardo Valentin sent to alsa-devel
mailing list in October 2009. Channel gain control and enabling the sidetone
have been moved to ALSA interface as suggested in comments. 

These patches apply on top of Janusz Krzysztofik's McBSP register cache
patches in linux-omap-2.6.git

Since the version 3, the buggy channel gain control has been fixed. ASoC part 
have been simplified resulting in removing a couple unnecessary functions.
Added a comment to ASoC part explaining McBSP port number vs. index values
used in OMAP McBSP implementation.

Eero Nurkkala (1):
  McBSP: OMAP3: Add sidetone feature

Ilkka Koskinen (1):
  ASoC: OMAP-McBSP: ASoC interface for McBSP sidetone

 arch/arm/mach-omap2/mcbsp.c             |    2 +
 arch/arm/plat-omap/include/plat/mcbsp.h |   60 +++++
 arch/arm/plat-omap/mcbsp.c              |  402 ++++++++++++++++++++++++++++++-
 sound/soc/omap/omap-mcbsp.c             |  138 +++++++++++
 sound/soc/omap/omap-mcbsp.h             |    2 +
 5 files changed, 603 insertions(+), 1 deletions(-)

[PATCHv4 1/2] McBSP: OMAP3: Add sidetone feature

[Ilkka Koskinen]

From: Eero Nurkkala <ext-eero.nurkkala@nokia.com>

Add sidetone feature to McBSP instances 2 and 3 on OMAP3 based devices.

Signed-off-by: Ilkka Koskinen <ilkka.koskinen@nokia.com>
---
 arch/arm/mach-omap2/mcbsp.c             |    2 +
 arch/arm/plat-omap/include/plat/mcbsp.h |   60 +++++
 arch/arm/plat-omap/mcbsp.c              |  402 ++++++++++++++++++++++++++++++-
 3 files changed, 463 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/mcbsp.c b/arch/arm/mach-omap2/mcbsp.c
index d601f94..be8fce3 100644
--- a/arch/arm/mach-omap2/mcbsp.c
+++ b/arch/arm/mach-omap2/mcbsp.c
@@ -136,6 +136,7 @@ static struct omap_mcbsp_platform_data omap34xx_mcbsp_pdata[] = {
 	},
 	{
 		.phys_base	= OMAP34XX_MCBSP2_BASE,
+		.phys_base_st	= OMAP34XX_MCBSP2_ST_BASE,
 		.dma_rx_sync	= OMAP24XX_DMA_MCBSP2_RX,
 		.dma_tx_sync	= OMAP24XX_DMA_MCBSP2_TX,
 		.rx_irq		= INT_24XX_MCBSP2_IRQ_RX,
@@ -145,6 +146,7 @@ static struct omap_mcbsp_platform_data omap34xx_mcbsp_pdata[] = {
 	},
 	{
 		.phys_base	= OMAP34XX_MCBSP3_BASE,
+		.phys_base_st	= OMAP34XX_MCBSP3_ST_BASE,
 		.dma_rx_sync	= OMAP24XX_DMA_MCBSP3_RX,
 		.dma_tx_sync	= OMAP24XX_DMA_MCBSP3_TX,
 		.rx_irq		= INT_24XX_MCBSP3_IRQ_RX,
diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h b/arch/arm/plat-omap/include/plat/mcbsp.h
index 4df957b..3974835 100644
--- a/arch/arm/plat-omap/include/plat/mcbsp.h
+++ b/arch/arm/plat-omap/include/plat/mcbsp.h
@@ -49,6 +49,9 @@
 
 #define OMAP34XX_MCBSP1_BASE	0x48074000
 #define OMAP34XX_MCBSP2_BASE	0x49022000
+#define OMAP34XX_MCBSP2_ST_BASE	0x49028000
+#define OMAP34XX_MCBSP3_BASE	0x49024000
+#define OMAP34XX_MCBSP3_ST_BASE	0x4902A000
 #define OMAP34XX_MCBSP3_BASE	0x49024000
 #define OMAP34XX_MCBSP4_BASE	0x49026000
 #define OMAP34XX_MCBSP5_BASE	0x48096000
@@ -146,6 +149,15 @@
 #define OMAP_MCBSP_REG_WAKEUPEN	0xA8
 #define OMAP_MCBSP_REG_XCCR	0xAC
 #define OMAP_MCBSP_REG_RCCR	0xB0
+#define OMAP_MCBSP_REG_SSELCR	0xBC
+
+#define OMAP_ST_REG_REV		0x00
+#define OMAP_ST_REG_SYSCONFIG	0x10
+#define OMAP_ST_REG_IRQSTATUS	0x18
+#define OMAP_ST_REG_IRQENABLE	0x1C
+#define OMAP_ST_REG_SGAINCR	0x24
+#define OMAP_ST_REG_SFIRCR	0x28
+#define OMAP_ST_REG_SSELCR	0x2C
 
 #define AUDIO_MCBSP_DATAWRITE	(OMAP24XX_MCBSP2_BASE + OMAP_MCBSP_REG_DXR1)
 #define AUDIO_MCBSP_DATAREAD	(OMAP24XX_MCBSP2_BASE + OMAP_MCBSP_REG_DRR1)
@@ -264,6 +276,24 @@
 #define ENAWAKEUP		0x0004
 #define SOFTRST			0x0002
 
+/********************** McBSP SSELCR bit definitions ***********************/
+#define SIDETONEEN		0x0400
+
+/********************** McBSP Sidetone SYSCONFIG bit definitions ***********/
+#define ST_AUTOIDLE		0x0001
+
+/********************** McBSP Sidetone SGAINCR bit definitions *************/
+#define ST_CH1GAIN(value)	((value<<16))	/* Bits 16:31 */
+#define ST_CH0GAIN(value)	(value)		/* Bits 0:15 */
+
+/********************** McBSP Sidetone SFIRCR bit definitions **************/
+#define ST_FIRCOEFF(value)	(value)		/* Bits 0:15 */
+
+/********************** McBSP Sidetone SSELCR bit definitions **************/
+#define ST_COEFFWRDONE		0x0004
+#define ST_COEFFWREN		0x0002
+#define ST_SIDETONEEN		0x0001
+
 /********************** McBSP DMA operating modes **************************/
 #define MCBSP_DMA_MODE_ELEMENT		0
 #define MCBSP_DMA_MODE_THRESHOLD	1
@@ -374,10 +404,22 @@ struct omap_mcbsp_platform_data {
 	u16 rx_irq, tx_irq;
 	struct omap_mcbsp_ops *ops;
 #ifdef CONFIG_ARCH_OMAP3
+	/* Sidetone block for McBSP 2 and 3 */
+	unsigned long phys_base_st;
 	u16 buffer_size;
 #endif
 };
 
+struct omap_mcbsp_st_data {
+	void __iomem *io_base_st;
+	bool running;
+	bool enabled;
+	s16 taps[128];	/* Sidetone filter coefficients */
+	int nr_taps;	/* Number of filter coefficients in use */
+	s16 ch0gain;
+	s16 ch1gain;
+};
+
 struct omap_mcbsp {
 	struct device *dev;
 	unsigned long phys_base;
@@ -410,6 +452,7 @@ struct omap_mcbsp {
 	struct clk *iclk;
 	struct clk *fclk;
 #ifdef CONFIG_ARCH_OMAP3
+	struct omap_mcbsp_st_data *st_data;
 	int dma_op_mode;
 	u16 max_tx_thres;
 	u16 max_rx_thres;
@@ -459,4 +502,21 @@ int omap_mcbsp_pollread(unsigned int id, u16 * buf);
 int omap_mcbsp_pollwrite(unsigned int id, u16 buf);
 int omap_mcbsp_set_io_type(unsigned int id, omap_mcbsp_io_type_t io_type);
 
+#ifdef CONFIG_ARCH_OMAP3
+/* Sidetone specific API */
+int omap_st_set_chgain(unsigned int id, int channel, s16 chgain);
+int omap_st_get_chgain(unsigned int id, int channel, s16 *chgain);
+int omap_st_enable(unsigned int id);
+int omap_st_disable(unsigned int id);
+int omap_st_is_enabled(unsigned int id);
+#else
+static inline int omap_st_set_chgain(unsigned int id, int channel,
+				     s16 chgain) { return 0; }
+static inline int omap_st_get_chgain(unsigned int id, int channel,
+				     s16 *chgain) { return 0; }
+static inline int omap_st_enable(unsigned int id) { return 0; }
+static inline int omap_st_disable(unsigned int id) { return 0; }
+static inline int omap_st_is_enabled(unsigned int id) {  return 0; }
+#endif
+
 #endif
diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index 473be3d..e47686e 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -27,6 +27,8 @@
 #include <plat/dma.h>
 #include <plat/mcbsp.h>
 
+#include "../mach-omap2/cm-regbits-34xx.h"
+
 struct omap_mcbsp **mcbsp_ptr;
 int omap_mcbsp_count, omap_mcbsp_cache_size;
 
@@ -58,6 +60,18 @@ int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg, bool from_cache)
 	}
 }
 
+#ifdef CONFIG_ARCH_OMAP3
+void omap_mcbsp_st_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
+{
+	__raw_writel(val, mcbsp->st_data->io_base_st + reg);
+}
+
+int omap_mcbsp_st_read(struct omap_mcbsp *mcbsp, u16 reg)
+{
+	return __raw_readl(mcbsp->st_data->io_base_st + reg);
+}
+#endif
+
 #define MCBSP_READ(mcbsp, reg) \
 		omap_mcbsp_read(mcbsp, OMAP_MCBSP_REG_##reg, 0)
 #define MCBSP_WRITE(mcbsp, reg, val) \
@@ -68,6 +82,11 @@ int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg, bool from_cache)
 #define omap_mcbsp_check_valid_id(id)	(id < omap_mcbsp_count)
 #define id_to_mcbsp_ptr(id)		mcbsp_ptr[id];
 
+#define MCBSP_ST_READ(mcbsp, reg) \
+			omap_mcbsp_st_read(mcbsp, OMAP_ST_REG_##reg)
+#define MCBSP_ST_WRITE(mcbsp, reg, val) \
+			omap_mcbsp_st_write(mcbsp, OMAP_ST_REG_##reg, val)
+
 static void omap_mcbsp_dump_reg(u8 id)
 {
 	struct omap_mcbsp *mcbsp = id_to_mcbsp_ptr(id);
@@ -211,6 +230,257 @@ void omap_mcbsp_config(unsigned int id, const struct omap_mcbsp_reg_cfg *config)
 EXPORT_SYMBOL(omap_mcbsp_config);
 
 #ifdef CONFIG_ARCH_OMAP3
+static void omap_st_on(struct omap_mcbsp *mcbsp)
+{
+	unsigned int w;
+
+	/*
+	 * Sidetone uses McBSP ICLK - which must not idle when sidetones
+	 * are enabled or sidetones start sounding ugly.
+	 */
+	w = cm_read_mod_reg(OMAP3430_PER_MOD, CM_AUTOIDLE);
+	w &= ~(1 << (mcbsp->id - 2));
+	cm_write_mod_reg(w, OMAP3430_PER_MOD, CM_AUTOIDLE);
+
+	/* Enable McBSP Sidetone */
+	w = MCBSP_READ(mcbsp, SSELCR);
+	MCBSP_WRITE(mcbsp, SSELCR, w | SIDETONEEN);
+
+	w = MCBSP_ST_READ(mcbsp, SYSCONFIG);
+	MCBSP_ST_WRITE(mcbsp, SYSCONFIG, w & ~(ST_AUTOIDLE));
+
+	/* Enable Sidetone from Sidetone Core */
+	w = MCBSP_ST_READ(mcbsp, SSELCR);
+	MCBSP_ST_WRITE(mcbsp, SSELCR, w | ST_SIDETONEEN);
+}
+
+static void omap_st_off(struct omap_mcbsp *mcbsp)
+{
+	unsigned int w;
+
+	w = MCBSP_ST_READ(mcbsp, SSELCR);
+	MCBSP_ST_WRITE(mcbsp, SSELCR, w & ~(ST_SIDETONEEN));
+
+	w = MCBSP_ST_READ(mcbsp, SYSCONFIG);
+	MCBSP_ST_WRITE(mcbsp, SYSCONFIG, w | ST_AUTOIDLE);
+
+	w = MCBSP_READ(mcbsp, SSELCR);
+	MCBSP_WRITE(mcbsp, SSELCR, w & ~(SIDETONEEN));
+
+	w = cm_read_mod_reg(OMAP3430_PER_MOD, CM_AUTOIDLE);
+	w |= 1 << (mcbsp->id - 2);
+	cm_write_mod_reg(w, OMAP3430_PER_MOD, CM_AUTOIDLE);
+}
+
+static void omap_st_fir_write(struct omap_mcbsp *mcbsp, s16 *fir)
+{
+	u16 val, i;
+
+	val = MCBSP_ST_READ(mcbsp, SYSCONFIG);
+	MCBSP_ST_WRITE(mcbsp, SYSCONFIG, val & ~(ST_AUTOIDLE));
+
+	val = MCBSP_ST_READ(mcbsp, SSELCR);
+
+	if (val & ST_COEFFWREN)
+		MCBSP_ST_WRITE(mcbsp, SSELCR, val & ~(ST_COEFFWREN));
+
+	MCBSP_ST_WRITE(mcbsp, SSELCR, val | ST_COEFFWREN);
+
+	for (i = 0; i < 128; i++)
+		MCBSP_ST_WRITE(mcbsp, SFIRCR, fir[i]);
+
+	i = 0;
+
+	val = MCBSP_ST_READ(mcbsp, SSELCR);
+	while (!(val & ST_COEFFWRDONE) && (++i < 1000))
+		val = MCBSP_ST_READ(mcbsp, SSELCR);
+
+	MCBSP_ST_WRITE(mcbsp, SSELCR, val & ~(ST_COEFFWREN));
+
+	if (i == 1000)
+		dev_err(mcbsp->dev, "McBSP FIR load error!\n");
+}
+
+static void omap_st_chgain(struct omap_mcbsp *mcbsp)
+{
+	u16 w;
+	struct omap_mcbsp_st_data *st_data = mcbsp->st_data;
+
+	w = MCBSP_ST_READ(mcbsp, SYSCONFIG);
+	MCBSP_ST_WRITE(mcbsp, SYSCONFIG, w & ~(ST_AUTOIDLE));
+
+	w = MCBSP_ST_READ(mcbsp, SSELCR);
+
+	MCBSP_ST_WRITE(mcbsp, SGAINCR, ST_CH0GAIN(st_data->ch0gain) | \
+		      ST_CH1GAIN(st_data->ch1gain));
+}
+
+int omap_st_set_chgain(unsigned int id, int channel, s16 chgain)
+{
+	struct omap_mcbsp *mcbsp;
+	struct omap_mcbsp_st_data *st_data;
+	int ret = 0;
+
+	if (!omap_mcbsp_check_valid_id(id)) {
+		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
+		return -ENODEV;
+	}
+
+	mcbsp = id_to_mcbsp_ptr(id);
+	st_data = mcbsp->st_data;
+
+	if (!st_data)
+		return -ENOENT;
+
+	spin_lock_irq(&mcbsp->lock);
+	if (channel == 0)
+		st_data->ch0gain = chgain;
+	else if (channel == 1)
+		st_data->ch1gain = chgain;
+	else
+		ret = -EINVAL;
+
+	if (st_data->enabled)
+		omap_st_chgain(mcbsp);
+	spin_unlock_irq(&mcbsp->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(omap_st_set_chgain);
+
+int omap_st_get_chgain(unsigned int id, int channel, s16 *chgain)
+{
+	struct omap_mcbsp *mcbsp;
+	struct omap_mcbsp_st_data *st_data;
+	int ret = 0;
+
+	if (!omap_mcbsp_check_valid_id(id)) {
+		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
+		return -ENODEV;
+	}
+
+	mcbsp = id_to_mcbsp_ptr(id);
+	st_data = mcbsp->st_data;
+
+	if (!st_data)
+		return -ENOENT;
+
+	spin_lock_irq(&mcbsp->lock);
+	if (channel == 0)
+		*chgain = st_data->ch0gain;
+	else if (channel == 1)
+		*chgain = st_data->ch1gain;
+	else
+		ret = -EINVAL;
+	spin_unlock_irq(&mcbsp->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(omap_st_get_chgain);
+
+static int omap_st_start(struct omap_mcbsp *mcbsp)
+{
+	struct omap_mcbsp_st_data *st_data = mcbsp->st_data;
+
+	if (st_data && st_data->enabled && !st_data->running) {
+		omap_st_fir_write(mcbsp, st_data->taps);
+		omap_st_chgain(mcbsp);
+
+		if (!mcbsp->free) {
+			omap_st_on(mcbsp);
+			st_data->running = 1;
+		}
+	}
+
+	return 0;
+}
+
+int omap_st_enable(unsigned int id)
+{
+	struct omap_mcbsp *mcbsp;
+	struct omap_mcbsp_st_data *st_data;
+
+	if (!omap_mcbsp_check_valid_id(id)) {
+		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
+		return -ENODEV;
+	}
+
+	mcbsp = id_to_mcbsp_ptr(id);
+	st_data = mcbsp->st_data;
+
+	if (!st_data)
+		return -ENODEV;
+
+	spin_lock_irq(&mcbsp->lock);
+	st_data->enabled = 1;
+	omap_st_start(mcbsp);
+	spin_unlock_irq(&mcbsp->lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(omap_st_enable);
+
+static int omap_st_stop(struct omap_mcbsp *mcbsp)
+{
+	struct omap_mcbsp_st_data *st_data = mcbsp->st_data;
+
+	if (st_data && st_data->running) {
+		if (!mcbsp->free) {
+			omap_st_off(mcbsp);
+			st_data->running = 0;
+		}
+	}
+
+	return 0;
+}
+
+int omap_st_disable(unsigned int id)
+{
+	struct omap_mcbsp *mcbsp;
+	struct omap_mcbsp_st_data *st_data;
+	int ret = 0;
+
+	if (!omap_mcbsp_check_valid_id(id)) {
+		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
+		return -ENODEV;
+	}
+
+	mcbsp = id_to_mcbsp_ptr(id);
+	st_data = mcbsp->st_data;
+
+	if (!st_data)
+		return -ENODEV;
+
+	spin_lock_irq(&mcbsp->lock);
+	omap_st_stop(mcbsp);
+	st_data->enabled = 0;
+	spin_unlock_irq(&mcbsp->lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(omap_st_disable);
+
+int omap_st_is_enabled(unsigned int id)
+{
+	struct omap_mcbsp *mcbsp;
+	struct omap_mcbsp_st_data *st_data;
+
+	if (!omap_mcbsp_check_valid_id(id)) {
+		printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
+		return -ENODEV;
+	}
+
+	mcbsp = id_to_mcbsp_ptr(id);
+	st_data = mcbsp->st_data;
+
+	if (!st_data)
+		return -ENODEV;
+
+
+	return st_data->enabled;
+}
+EXPORT_SYMBOL(omap_st_is_enabled);
+
 /*
  * omap_mcbsp_set_tx_threshold configures how to deal
  * with transmit threshold. the threshold value and handler can be
@@ -363,6 +633,8 @@ static inline void omap34xx_mcbsp_free(struct omap_mcbsp *mcbsp)
 #else
 static inline void omap34xx_mcbsp_request(struct omap_mcbsp *mcbsp) {}
 static inline void omap34xx_mcbsp_free(struct omap_mcbsp *mcbsp) {}
+static inline void omap_st_start(struct omap_mcbsp *mcbsp) {}
+static inline void omap_st_stop(struct omap_mcbsp *mcbsp) {}
 #endif
 
 /*
@@ -546,6 +818,9 @@ void omap_mcbsp_start(unsigned int id, int tx, int rx)
 	}
 	mcbsp = id_to_mcbsp_ptr(id);
 
+	if (cpu_is_omap34xx())
+		omap_st_start(mcbsp);
+
 	mcbsp->rx_word_length = (MCBSP_READ_CACHE(mcbsp, RCR1) >> 5) & 0x7;
 	mcbsp->tx_word_length = (MCBSP_READ_CACHE(mcbsp, XCR1) >> 5) & 0x7;
 
@@ -637,6 +912,9 @@ void omap_mcbsp_stop(unsigned int id, int tx, int rx)
 		w = MCBSP_READ_CACHE(mcbsp, SPCR2);
 		MCBSP_WRITE(mcbsp, SPCR2, w & ~(1 << 6));
 	}
+
+	if (cpu_is_omap34xx())
+		omap_st_stop(mcbsp);
 }
 EXPORT_SYMBOL(omap_mcbsp_stop);
 
@@ -1212,6 +1490,64 @@ unlock:
 
 static DEVICE_ATTR(dma_op_mode, 0644, dma_op_mode_show, dma_op_mode_store);
 
+static ssize_t st_taps_show(struct device *dev,
+			    struct device_attribute *attr, char *buf)
+{
+	struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
+	struct omap_mcbsp_st_data *st_data = mcbsp->st_data;
+	ssize_t status = 0;
+	int i;
+
+	spin_lock_irq(&mcbsp->lock);
+	for (i = 0; i < st_data->nr_taps; i++)
+		status += sprintf(&buf[status], (i ? ", %d" : "%d"),
+				  st_data->taps[i]);
+	if (i)
+		status += sprintf(&buf[status], "\n");
+	spin_unlock_irq(&mcbsp->lock);
+
+	return status;
+}
+
+static ssize_t st_taps_store(struct device *dev,
+			     struct device_attribute *attr,
+			     const char *buf, size_t size)
+{
+	struct omap_mcbsp *mcbsp = dev_get_drvdata(dev);
+	struct omap_mcbsp_st_data *st_data = mcbsp->st_data;
+	int val, tmp, status, i = 0;
+
+	spin_lock_irq(&mcbsp->lock);
+	memset(st_data->taps, 0, sizeof(st_data->taps));
+	st_data->nr_taps = 0;
+
+	do {
+		status = sscanf(buf, "%d%n", &val, &tmp);
+		if (status < 0 || status == 0) {
+			size = -EINVAL;
+			goto out;
+		}
+		if (val < -32768 || val > 32767) {
+			size = -EINVAL;
+			goto out;
+		}
+		st_data->taps[i++] = val;
+		buf += tmp;
+		if (*buf != ',')
+			break;
+		buf++;
+	} while (1);
+
+	st_data->nr_taps = i;
+
+out:
+	spin_unlock_irq(&mcbsp->lock);
+
+	return size;
+}
+
+static DEVICE_ATTR(st_taps, 0644, st_taps_show, st_taps_store);
+
 static const struct attribute *additional_attrs[] = {
 	&dev_attr_max_tx_thres.attr,
 	&dev_attr_max_rx_thres.attr,
@@ -1233,6 +1569,60 @@ static inline void __devexit omap_additional_remove(struct device *dev)
 	sysfs_remove_group(&dev->kobj, &additional_attr_group);
 }
 
+static const struct attribute *sidetone_attrs[] = {
+	&dev_attr_st_taps.attr,
+	NULL,
+};
+
+static const struct attribute_group sidetone_attr_group = {
+	.attrs = (struct attribute **)sidetone_attrs,
+};
+
+int __devinit omap_st_add(struct omap_mcbsp *mcbsp)
+{
+	struct omap_mcbsp_platform_data *pdata = mcbsp->pdata;
+	struct omap_mcbsp_st_data *st_data;
+	int err;
+
+	st_data = kzalloc(sizeof(*mcbsp->st_data), GFP_KERNEL);
+	if (!st_data) {
+		err = -ENOMEM;
+		goto err1;
+	}
+
+	st_data->io_base_st = ioremap(pdata->phys_base_st, SZ_4K);
+	if (!st_data->io_base_st) {
+		err = -ENOMEM;
+		goto err2;
+	}
+
+	err = sysfs_create_group(&mcbsp->dev->kobj, &sidetone_attr_group);
+	if (err)
+		goto err3;
+
+	mcbsp->st_data = st_data;
+	return 0;
+
+err3:
+	iounmap(st_data->io_base_st);
+err2:
+	kfree(st_data);
+err1:
+	return err;
+
+}
+
+static void __devexit omap_st_remove(struct omap_mcbsp *mcbsp)
+{
+	struct omap_mcbsp_st_data *st_data = mcbsp->st_data;
+
+	if (st_data) {
+		sysfs_remove_group(&mcbsp->dev->kobj, &sidetone_attr_group);
+		iounmap(st_data->io_base_st);
+		kfree(st_data);
+	}
+}
+
 static inline void __devinit omap34xx_device_init(struct omap_mcbsp *mcbsp)
 {
 	mcbsp->dma_op_mode = MCBSP_DMA_MODE_ELEMENT;
@@ -1246,6 +1636,12 @@ static inline void __devinit omap34xx_device_init(struct omap_mcbsp *mcbsp)
 		if (omap_additional_add(mcbsp->dev))
 			dev_warn(mcbsp->dev,
 				"Unable to create additional controls\n");
+
+		if (mcbsp->id == 2 || mcbsp->id == 3)
+			if (omap_st_add(mcbsp))
+				dev_warn(mcbsp->dev,
+				 "Unable to create sidetone controls\n");
+
 	} else {
 		mcbsp->max_tx_thres = -EINVAL;
 		mcbsp->max_rx_thres = -EINVAL;
@@ -1254,8 +1650,12 @@ static inline void __devinit omap34xx_device_init(struct omap_mcbsp *mcbsp)
 
 static inline void __devexit omap34xx_device_exit(struct omap_mcbsp *mcbsp)
 {
-	if (cpu_is_omap34xx())
+	if (cpu_is_omap34xx()) {
 		omap_additional_remove(mcbsp->dev);
+
+		if (mcbsp->id == 2 || mcbsp->id == 3)
+			omap_st_remove(mcbsp);
+	}
 }
 #else
 static inline void __devinit omap34xx_device_init(struct omap_mcbsp *mcbsp) {}
-- 
1.6.0.4

[PATCHv4 2/2] ASoC: OMAP-McBSP: ASoC interface for McBSP sidetone

[Ilkka Koskinen]

Add ASoC interface for OMAP McBSP2 and McBSP3 sidetones.

Signed-off-by: Ilkka Koskinen <ilkka.koskinen@nokia.com>
---
 sound/soc/omap/omap-mcbsp.c |  138 +++++++++++++++++++++++++++++++++++++++++++
 sound/soc/omap/omap-mcbsp.h |    2 +
 2 files changed, 140 insertions(+), 0 deletions(-)

diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
index c0039b3..503c58d 100644
--- a/sound/soc/omap/omap-mcbsp.c
+++ b/sound/soc/omap/omap-mcbsp.c
@@ -39,6 +39,14 @@
 
 #define OMAP_MCBSP_RATES	(SNDRV_PCM_RATE_8000_96000)
 
+#define OMAP_MCBSP_SOC_SINGLE_S16_EXT(xname, xmin, xmax, \
+	xhandler_get, xhandler_put) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.info = omap_mcbsp_st_info_volsw, \
+	.get = xhandler_get, .put = xhandler_put, \
+	.private_value = (unsigned long) &(struct soc_mixer_control) \
+	{.min = xmin, .max = xmax} }
+
 struct omap_mcbsp_data {
 	unsigned int			bus_id;
 	struct omap_mcbsp_reg_cfg	regs;
@@ -637,6 +645,136 @@ struct snd_soc_dai omap_mcbsp_dai[] = {
 
 EXPORT_SYMBOL_GPL(omap_mcbsp_dai);
 
+int omap_mcbsp_st_info_volsw(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_info *uinfo)
+{
+	struct soc_mixer_control *mc =
+		(struct soc_mixer_control *)kcontrol->private_value;
+	int max = mc->max;
+	int min = mc->min;
+
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 1;
+	uinfo->value.integer.min = min;
+	uinfo->value.integer.max = max;
+	return 0;
+}
+
+#define OMAP_MCBSP_ST_SET_CHANNEL_VOLUME(id, channel)			\
+static int								\
+omap_mcbsp##id##_set_st_ch##channel##_volume(struct snd_kcontrol *kc,	\
+					struct snd_ctl_elem_value *uc)	\
+{									\
+	struct soc_mixer_control *mc =					\
+		(struct soc_mixer_control *)kc->private_value;		\
+	int max = mc->max;						\
+	int min = mc->min;						\
+	int val = uc->value.integer.value[0];				\
+									\
+	if (val < min || val > max)					\
+		return -EINVAL;						\
+									\
+	/* OMAP McBSP implementation uses index values 0..4 */		\
+	return omap_st_set_chgain((id)-1, channel, val);		\
+}
+
+#define OMAP_MCBSP_ST_GET_CHANNEL_VOLUME(id, channel)			\
+static int								\
+omap_mcbsp##id##_get_st_ch##channel##_volume(struct snd_kcontrol *kc,	\
+					struct snd_ctl_elem_value *uc)	\
+{									\
+	s16 chgain;							\
+									\
+	if (omap_st_get_chgain((id)-1, channel, &chgain))		\
+		return -EAGAIN;						\
+									\
+	uc->value.integer.value[0] = chgain;				\
+	return 0;							\
+}
+
+OMAP_MCBSP_ST_SET_CHANNEL_VOLUME(2, 0)
+OMAP_MCBSP_ST_SET_CHANNEL_VOLUME(2, 1)
+OMAP_MCBSP_ST_SET_CHANNEL_VOLUME(3, 0)
+OMAP_MCBSP_ST_SET_CHANNEL_VOLUME(3, 1)
+OMAP_MCBSP_ST_GET_CHANNEL_VOLUME(2, 0)
+OMAP_MCBSP_ST_GET_CHANNEL_VOLUME(2, 1)
+OMAP_MCBSP_ST_GET_CHANNEL_VOLUME(3, 0)
+OMAP_MCBSP_ST_GET_CHANNEL_VOLUME(3, 1)
+
+static int omap_mcbsp_st_put_mode(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_mixer_control *mc =
+		(struct soc_mixer_control *)kcontrol->private_value;
+	u8 value = ucontrol->value.integer.value[0];
+
+	if (value == omap_st_is_enabled(mc->reg))
+		return 0;
+
+	if (value)
+		omap_st_enable(mc->reg);
+	else
+		omap_st_disable(mc->reg);
+
+	return 1;
+}
+
+static int omap_mcbsp_st_get_mode(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_mixer_control *mc =
+		(struct soc_mixer_control *)kcontrol->private_value;
+
+	ucontrol->value.integer.value[0] = omap_st_is_enabled(mc->reg);
+	return 0;
+}
+
+static const struct snd_kcontrol_new omap_mcbsp2_st_controls[] = {
+	SOC_SINGLE_EXT("McBSP2 Sidetone Switch", 1, 0, 1, 0,
+			omap_mcbsp_st_get_mode, omap_mcbsp_st_put_mode),
+	OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP2 Sidetone Channel 0 Volume",
+				      -32768, 32767,
+				      omap_mcbsp2_get_st_ch0_volume,
+				      omap_mcbsp2_set_st_ch0_volume),
+	OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP2 Sidetone Channel 1 Volume",
+				      -32768, 32767,
+				      omap_mcbsp2_get_st_ch1_volume,
+				      omap_mcbsp2_set_st_ch1_volume),
+};
+
+static const struct snd_kcontrol_new omap_mcbsp3_st_controls[] = {
+	SOC_SINGLE_EXT("McBSP3 Sidetone Switch", 2, 0, 1, 0,
+			omap_mcbsp_st_get_mode, omap_mcbsp_st_put_mode),
+	OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP3 Sidetone Channel 0 Volume",
+				      -32768, 32767,
+				      omap_mcbsp3_get_st_ch0_volume,
+				      omap_mcbsp3_set_st_ch0_volume),
+	OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP3 Sidetone Channel 1 Volume",
+				      -32768, 32767,
+				      omap_mcbsp3_get_st_ch1_volume,
+				      omap_mcbsp3_set_st_ch1_volume),
+};
+
+int omap_mcbsp_st_add_controls(struct snd_soc_codec *codec, int mcbsp_id)
+{
+	if (!cpu_is_omap34xx())
+		return -ENODEV;
+
+	switch (mcbsp_id) {
+	case 2: /* McBSP 2 */
+		return snd_soc_add_controls(codec, omap_mcbsp2_st_controls,
+					ARRAY_SIZE(omap_mcbsp2_st_controls));
+	case 3: /* McBSP 3 */
+		return snd_soc_add_controls(codec, omap_mcbsp3_st_controls,
+					ARRAY_SIZE(omap_mcbsp3_st_controls));
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(omap_mcbsp_st_add_controls);
+
 static int __init snd_omap_mcbsp_init(void)
 {
 	return snd_soc_register_dais(omap_mcbsp_dai,
diff --git a/sound/soc/omap/omap-mcbsp.h b/sound/soc/omap/omap-mcbsp.h
index 1968d03..6c363e5 100644
--- a/sound/soc/omap/omap-mcbsp.h
+++ b/sound/soc/omap/omap-mcbsp.h
@@ -57,4 +57,6 @@ enum omap_mcbsp_div {
 
 extern struct snd_soc_dai omap_mcbsp_dai[NUM_LINKS];
 
+int omap_mcbsp_st_add_controls(struct snd_soc_codec *codec, int mcbsp_id);
+
 #endif
-- 
1.6.0.4

RE: [PATCHv4 2/2] ASoC: OMAP-McBSP: ASoC interface for McBSP sidetone

[ext-Eero.Nurkkala]

> +
> +static const struct snd_kcontrol_new omap_mcbsp2_st_controls[] = {
> +       SOC_SINGLE_EXT("McBSP2 Sidetone Switch", 1, 0, 1, 0,
> +                       omap_mcbsp_st_get_mode, omap_mcbsp_st_put_mode),
> +       OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP2 Sidetone Channel 0 Volume",
> +                                     -32768, 32767,
> +                                     omap_mcbsp2_get_st_ch0_volume,
> +                                     omap_mcbsp2_set_st_ch0_volume),
> +       OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP2 Sidetone Channel 1 Volume",
> +                                     -32768, 32767,
> +                                     omap_mcbsp2_get_st_ch1_volume,
> +                                     omap_mcbsp2_set_st_ch1_volume),
> +};
> +
> +static const struct snd_kcontrol_new omap_mcbsp3_st_controls[] = {
> +       SOC_SINGLE_EXT("McBSP3 Sidetone Switch", 2, 0, 1, 0,
> +                       omap_mcbsp_st_get_mode, omap_mcbsp_st_put_mode),
> +       OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP3 Sidetone Channel 0 Volume",
> +                                     -32768, 32767,
> +                                     omap_mcbsp3_get_st_ch0_volume,
> +                                     omap_mcbsp3_set_st_ch0_volume),
> +       OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP3 Sidetone Channel 1 Volume",
> +                                     -32768, 32767,
> +                                     omap_mcbsp3_get_st_ch1_volume,
> +                                     omap_mcbsp3_set_st_ch1_volume),
> +};
> +

Just a short note,
"Gain values are in the interval (-2..2) in Q1.14 format, negative values are expressed in
2's complement."

So, from:
http://en.wikipedia.org/wiki/Q_(number_format)

"
The notation used is Qm.n, where:

    * Q designates that the number is in Q format notation — the Texas Instruments representation for signed fixed-point numbers.
    * m (optional; default=0) is the number of bits used to designate the two's complement integer portion of the number, exclusive of the sign bit.
    * n is the number of bits used to designate the two's complement fractional portion of the number, i.e. the number of bits to the right of the binary point.
"

..so if you do tests, and find out the gains behave "irrationally" occasionally, you can't possibly claim it isn't working as expected ;)

- Eero--
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: [PATCHv4 2/2] ASoC: OMAP-McBSP: ASoC interface for McBSP sidetone

[Ilkka Koskinen]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2764 bytes --]



On Fri, 19 Feb 2010, Nurkkala Eero.An (EXT-Offcode/Oulu) wrote:

>
>> +
>> +static const struct snd_kcontrol_new omap_mcbsp2_st_controls[] = {
>> +       SOC_SINGLE_EXT("McBSP2 Sidetone Switch", 1, 0, 1, 0,
>> +                       omap_mcbsp_st_get_mode, omap_mcbsp_st_put_mode),
>> +       OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP2 Sidetone Channel 0 Volume",
>> +                                     -32768, 32767,
>> +                                     omap_mcbsp2_get_st_ch0_volume,
>> +                                     omap_mcbsp2_set_st_ch0_volume),
>> +       OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP2 Sidetone Channel 1 Volume",
>> +                                     -32768, 32767,
>> +                                     omap_mcbsp2_get_st_ch1_volume,
>> +                                     omap_mcbsp2_set_st_ch1_volume),
>> +};
>> +
>> +static const struct snd_kcontrol_new omap_mcbsp3_st_controls[] = {
>> +       SOC_SINGLE_EXT("McBSP3 Sidetone Switch", 2, 0, 1, 0,
>> +                       omap_mcbsp_st_get_mode, omap_mcbsp_st_put_mode),
>> +       OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP3 Sidetone Channel 0 Volume",
>> +                                     -32768, 32767,
>> +                                     omap_mcbsp3_get_st_ch0_volume,
>> +                                     omap_mcbsp3_set_st_ch0_volume),
>> +       OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP3 Sidetone Channel 1 Volume",
>> +                                     -32768, 32767,
>> +                                     omap_mcbsp3_get_st_ch1_volume,
>> +                                     omap_mcbsp3_set_st_ch1_volume),
>> +};
>> +
>
> Just a short note, "Gain values are in the interval (-2..2) in Q1.14 
> format, negative values are expressed in 2's complement."
>
> So, from:
> http://en.wikipedia.org/wiki/Q_(number_format)
>
> "
> The notation used is Qm.n, where:
>
>    * Q designates that the number is in Q format notation — the Texas 
> Instruments representation for signed fixed-point numbers.
>    * m (optional; default=0) is the number of bits used to designate the 
> two's complement integer portion of the number, exclusive of the sign 
> bit.
>    * n is the number of bits used to designate the two's complement 
> fractional portion of the number, i.e. the number of bits to the right 
> of the binary point. "
>
> ..so if you do tests, and find out the gains behave "irrationally" 
> occasionally, you can't possibly claim it isn't working as expected ;)

I'm a bit confused. What do you mean by that?

AFAICS, Q1.14 defines values [-16384.0, +16383.5], which are mapped in the 
driver to integers [-32768, 32767], right? Moreover, those Q1.14 values in 
registers are mapped to [-2, 2] in HW. So is there a problem somewhere?

Cheers, Ilkka

Re: [PATCHv4 2/2] ASoC: OMAP-McBSP: ASoC interface for McBSP sidetone

[ext-Eero.Nurkkala]

> I'm a bit confused. What do you mean by that?
> 
> AFAICS, Q1.14 defines values [-16384.0, +16383.5], which are mapped in the
> driver to integers [-32768, 32767], right? Moreover, those Q1.14 values in
> registers are mapped to [-2, 2] in HW. So is there a problem somewhere?
> 
> Cheers, Ilkka

No, don't get me wrong, not saying there's a problem with the positive gains,
Q1.14: its range is [-16384.0, +16383.5] = [0x8000, 0x8001 … 0xFFFF,
0x0000, 0x0001 … 0x7FFE, 0x7FFF]
if I read this correctly, your min is 0xFFFF, but the real min maybe
0x8000? To be honest, it may be just me interpreting that incorrectly..
...as if the value is in two's complement, it goes the opposite:
http://en.wikipedia.org/wiki/Two%27s_complement , see:
1 0 0 0 0 0 0 0 = −128
1 1 1 1 1 1 1 1 = −1
what do you think? maybe try it out to see the truth..

- Eero
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

RE: [PATCHv4 2/2] ASoC: OMAP-McBSP: ASoC interface for McBSP sidetone

[ilkka.koskinen]

 

>From: Nurkkala Eero.An (EXT-Offcode/Oulu) 
>Sent: 19 February, 2010 14:58
>
>> I'm a bit confused. What do you mean by that?
>> 
>> AFAICS, Q1.14 defines values [-16384.0, +16383.5], which are 
>mapped in the
>> driver to integers [-32768, 32767], right? Moreover, those 
>Q1.14 values in
>> registers are mapped to [-2, 2] in HW. So is there a problem 
>somewhere?
>> 
>> Cheers, Ilkka
>
>No, don't get me wrong, not saying there's a problem with the 
>positive gains,
>Q1.14: its range is [-16384.0, +16383.5] = [0x8000, 0x8001 … 0xFFFF,
>0x0000, 0x0001 … 0x7FFE, 0x7FFF]
>if I read this correctly, your min is 0xFFFF, but the real min maybe
>0x8000? To be honest, it may be just me interpreting that incorrectly..
>...as if the value is in two's complement, it goes the opposite:
>http://en.wikipedia.org/wiki/Two%27s_complement , see:
>1 0 0 0 0 0 0 0 = −128
>1 1 1 1 1 1 1 1 = −1
>what do you think? maybe try it out to see the truth..

Oops, I seemed to copy Q1.14 instead of Q14.1, but isn't it still the same.
When I added some debug messages it still seemed to be corret.

Lowest number (-32768) is represented with 16th bit '1' and the rest are
zeros, right? That is 0x8000.
-1 has all the bits set (0xffff) and 0 has all the bit cleared (0x0000).
Highest positive value has 16th bit cleared and the rest set (0x7fff).

Or did I interpret something wrong?

Cheers, Ilkka

Re: [PATCHv4 2/2] ASoC: OMAP-McBSP: ASoC interface for McBSP sidetone

[ext-Eero.Nurkkala]

> Oops, I seemed to copy Q1.14 instead of Q14.1, but isn't it still the same.
> When I added some debug messages it still seemed to be corret.
> 
> Lowest number (-32768) is represented with 16th bit '1' and the rest are
> zeros, right? That is 0x8000.
> -1 has all the bits set (0xffff) and 0 has all the bit cleared (0x0000).
> Highest positive value has 16th bit cleared and the rest set (0x7fff).
> 
> Or did I interpret something wrong?
> 
> Cheers, Ilkka

I guess it's just fine, but let's see. Maybe I was lost in the Q1.14: m + n + 1 ~
binary: [sign bit, (m), (n)] where m is the integer portion, 0 or 1, n is 14 bits..
so  if your input was [-32768... 32767] -> [-2,2] then, for example,
-32768 is in hex: 0x8000, but the 2nd most significant bit is zero, which means
the integer portion (m) is not 1, which makes me doubt the gain -32768 is
actually -1 (or 0), not -2. But then, as it's a two's complement, it maybe just correct.

So most likely it's just fine; I just had a thinko.

- Eero

RE: [PATCHv4 2/2] ASoC: OMAP-McBSP: ASoC interface for McBSP sidetone

[ilkka.koskinen]

 

>From: Nurkkala Eero.An (EXT-Offcode/Oulu) 
>Sent: 19 February, 2010 21:04
>
>> Oops, I seemed to copy Q1.14 instead of Q14.1, but isn't it 
>still the same.
>> When I added some debug messages it still seemed to be corret.
>> 
>> Lowest number (-32768) is represented with 16th bit '1' and 
>the rest are
>> zeros, right? That is 0x8000.
>> -1 has all the bits set (0xffff) and 0 has all the bit 
>cleared (0x0000).
>> Highest positive value has 16th bit cleared and the rest set 
>(0x7fff).
>> 
>> Or did I interpret something wrong?
>> 
>> Cheers, Ilkka
>
>I guess it's just fine, but let's see. Maybe I was lost in the 
>Q1.14: m + n + 1 ~
>binary: [sign bit, (m), (n)] where m is the integer portion, 0 
>or 1, n is 14 bits..
>so  if your input was [-32768... 32767] -> [-2,2] then, for example,
>-32768 is in hex: 0x8000, but the 2nd most significant bit is 
>zero, which means
>the integer portion (m) is not 1, which makes me doubt the 
>gain -32768 is
>actually -1 (or 0), not -2. But then, as it's a two's 
>complement, it maybe just correct.
>
>So most likely it's just fine; I just had a thinko.

Ok, now I got your point :) However, as far as I can understand
the Q notation right, it basically defines how to interpret the 
value. That is, where is the decimal point. Thus, I would say that
the code should work fine and based on quick tests, the gain control
seems to behave ok too.

Cheers, Ilkka

Re: [PATCHv4 0/2] McBSP: OMAP3: Add sidetone feature

[Jarkko Nikula]

On Thu, 18 Feb 2010 20:42:27 +0200
Ilkka Koskinen <ilkka.koskinen@nokia.com> wrote:

> The first patch implements McBSP sidetone feature on OMAP3 while the second
> one provides ALSA interface for using it.
> 
> The patch set is based on the patch Eduardo Valentin sent to alsa-devel
> mailing list in October 2009. Channel gain control and enabling the sidetone
> have been moved to ALSA interface as suggested in comments. 
> 
> These patches apply on top of Janusz Krzysztofik's McBSP register cache
> patches in linux-omap-2.6.git
> 
> Since the version 3, the buggy channel gain control has been fixed. ASoC part 
> have been simplified resulting in removing a couple unnecessary functions.
> Added a comment to ASoC part explaining McBSP port number vs. index values
> used in OMAP McBSP implementation.
> 
I still have two concern:

1. port number vs. index difference inside driver code is not enough

It's just confusing if a machine driver and internal API (mcbsp.c) has
to use different indexing. See:

omap_mcbsp_st_add_controls(codec, 2);
...
*(unsigned int *)omap3beagle_dai.cpu_dai->private_data = 1; /* McBSP2 */

and mcbsp->id use in mcbsp.c.

2. I cannot change the sidetone level with alsamixer

Is this bug with the alsamixer? With amixer I'm able to set positive
values but not negative

amixer set -D hw:0 'McBSP2 Sidetone Channel 0' 32767

What I'm looking here, is that I can execute a test below with unit
impulse response tap filter, hear the sound and note that the volume is
changing as I'm changing the sidetone volume :-)

arecord -f dat >/dev/null |aplay -f dat /dev/zero


-- 
Jarkko

Re: [PATCHv4 0/2] McBSP: OMAP3: Add sidetone feature

[Peter Ujfalusi]

On Thursday 18 February 2010 22:22:29 ext Jarkko Nikula wrote:
> 
> I still have two concern:
> 
> 1. port number vs. index difference inside driver code is not enough
> 
> It's just confusing if a machine driver and internal API (mcbsp.c) has
> to use different indexing. See:
> 
> omap_mcbsp_st_add_controls(codec, 2);
> ...
> *(unsigned int *)omap3beagle_dai.cpu_dai->private_data = 1; /* McBSP2 */

Agreed.

 
> and mcbsp->id use in mcbsp.c.

It has always bothered me, I'll take a look and check if there is a reason why 
it is like that, and if possible I'll change the mcbsp->id to be 0 based (thus 
harmonizing the internal and external interface).
 
> 2. I cannot change the sidetone level with alsamixer
> 
> Is this bug with the alsamixer? With amixer I'm able to set positive
> values but not negative
> 
> amixer set -D hw:0 'McBSP2 Sidetone Channel 0' 32767
> 
> What I'm looking here, is that I can execute a test below with unit
> impulse response tap filter, hear the sound and note that the volume is
> changing as I'm changing the sidetone volume :-)

I'm not sure, but you might need to configure the taps as well to have the 
correct sidetone.
Also I have a recollection, that the McBSP sidetone requires DSP mode (TDM) in 
order to operate correctly, so on Beagle you might need to set up 4 channel 
mode. And how the channel mapping in 4 channel mode works in McBSP sidetone is 
another matter...

> 
> arecord -f dat >/dev/null |aplay -f dat /dev/zero

-- 
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

Re: [PATCHv4 0/2] McBSP: OMAP3: Add sidetone feature

[Jarkko Nikula]

On Fri, 19 Feb 2010 10:34:32 +0200
Peter Ujfalusi <peter.ujfalusi@nokia.com> wrote:

> I'm not sure, but you might need to configure the taps as well to have the 
> correct sidetone.
> Also I have a recollection, that the McBSP sidetone requires DSP mode (TDM) in 
> order to operate correctly, so on Beagle you might need to set up 4 channel 
> mode. And how the channel mapping in 4 channel mode works in McBSP sidetone is 
> another matter...
> 
It seems to work with I2S too.

I think I forgot to enable control 'McBSP2 Sidetone' when I last time
tried this set (v1 or v2). But now my null test below works and I can
hear that the level is changing when I adjust the volumes with amixer.

echo 32767 >/sys/devices/platform/omap-mcbsp.2/st_taps

arecord -f dat >/dev/null |aplay -f dat /dev/zero

amixer set -D hw:0 'McBSP2 Sidetone Channel 0' 32700
amixer set -D hw:0 'McBSP2 Sidetone Channel 0' 327
amixer set -D hw:0 'McBSP2 Sidetone Channel 0' -- -32700


-- 
Jarkko

Re: [PATCHv4 0/2] McBSP: OMAP3: Add sidetone feature

[Eero Nurkkala]

On Fri, 2010-02-19 at 14:39 +0100, ext Jarkko Nikula wrote:
> On Fri, 19 Feb 2010 10:34:32 +0200
> Peter Ujfalusi <peter.ujfalusi@nokia.com> wrote:
> 
> > I'm not sure, but you might need to configure the taps as well to have the 
> > correct sidetone.
> > Also I have a recollection, that the McBSP sidetone requires DSP mode (TDM) in 
> > order to operate correctly, so on Beagle you might need to set up 4 channel 
> > mode. And how the channel mapping in 4 channel mode works in McBSP sidetone is 
> > another matter...
> > 
> It seems to work with I2S too.
> 

For both channels? I don't recall so.. and for the one channel only, the
FIR doesn't behave expectedly? (maybe this works [1,0,..0], but more
sophisticated ones may not..)

> I think I forgot to enable control 'McBSP2 Sidetone' when I last time
> tried this set (v1 or v2). But now my null test below works and I can
> hear that the level is changing when I adjust the volumes with amixer.
> 
> echo 32767 >/sys/devices/platform/omap-mcbsp.2/st_taps
> 
> arecord -f dat >/dev/null |aplay -f dat /dev/zero
> 
> amixer set -D hw:0 'McBSP2 Sidetone Channel 0' 32700
> amixer set -D hw:0 'McBSP2 Sidetone Channel 0' 327
> amixer set -D hw:0 'McBSP2 Sidetone Channel 0' -- -32700
> 
> 

Re: [PATCHv4 0/2] McBSP: OMAP3: Add sidetone feature

[Jarkko Nikula]

On Fri, 19 Feb 2010 15:37:17 +0200
Eero Nurkkala <ext-eero.nurkkala@nokia.com> wrote:

> > It seems to work with I2S too.
> > 
> 
> For both channels? I don't recall so.. and for the one channel only, the
> FIR doesn't behave expectedly? (maybe this works [1,0,..0], but more
> sophisticated ones may not..)
> 
Could be. At least the control 'McBSP2 Sidetone Channel 1' doesn't have
any effect. But the control 0 is affecting both channels
simultaneously. (I didn't check the meaning of those two controls)


-- 
Jarkko

Re: [PATCHv4 0/2] McBSP: OMAP3: Add sidetone feature

[Ilkka Koskinen]

On Thu, 18 Feb 2010, ext Jarkko Nikula wrote:
> On Thu, 18 Feb 2010 20:42:27 +0200
> Ilkka Koskinen <ilkka.koskinen@nokia.com> wrote:
>
>> The first patch implements McBSP sidetone feature on OMAP3 while the second
>> one provides ALSA interface for using it.
>>
>> The patch set is based on the patch Eduardo Valentin sent to alsa-devel
>> mailing list in October 2009. Channel gain control and enabling the sidetone
>> have been moved to ALSA interface as suggested in comments.
>>
>> These patches apply on top of Janusz Krzysztofik's McBSP register cache
>> patches in linux-omap-2.6.git
>>
>> Since the version 3, the buggy channel gain control has been fixed. ASoC part
>> have been simplified resulting in removing a couple unnecessary functions.
>> Added a comment to ASoC part explaining McBSP port number vs. index values
>> used in OMAP McBSP implementation.
>>
> I still have two concern:
>
> 1. port number vs. index difference inside driver code is not enough
>
> It's just confusing if a machine driver and internal API (mcbsp.c) has
> to use different indexing. See:
>
> omap_mcbsp_st_add_controls(codec, 2);
> ...
> *(unsigned int *)omap3beagle_dai.cpu_dai->private_data = 1; /* McBSP2 */
>
> and mcbsp->id use in mcbsp.c.

Ah, that's what you meant by it. Sure, I'll change it.

> 2. I cannot change the sidetone level with alsamixer
>
I think with the previous patch set, it was impossible to set the other 
channel's gain as Peter pointed out. But at least now that I try, i can 
change it with alsamixer.

> Is this bug with the alsamixer? With amixer I'm able to set positive
> values but not negative
>
> amixer set -D hw:0 'McBSP2 Sidetone Channel 0' 32767

If you try to give negative values, you need to put '--' before the value.

Such as:

# amixer -Dhw:0 -- sset 'McBSP3 Sidetone Channel 0' -10000
Simple mixer control 'McBSP3 Sidetone Channel 0',0
   Capabilities: volume volume-joined
   Playback channels: Mono
   Capture channels: Mono
   Limits: -32768 - 32767
   Mono: -10000 [35%]


> What I'm looking here, is that I can execute a test below with unit
> impulse response tap filter, hear the sound and note that the volume is
> changing as I'm changing the sidetone volume :-)
>
> arecord -f dat >/dev/null |aplay -f dat /dev/zero

As Peter said in the other mail, you need to have the taps also.

Cheers, Ilkka

Re: [PATCHv4 0/2] McBSP: OMAP3: Add sidetone feature

[Jarkko Nikula]

On Fri, 19 Feb 2010 12:05:40 +0200 (EET)
Ilkka Koskinen <ikoskine@nokia.com> wrote:

> > 2. I cannot change the sidetone level with alsamixer
> >
> I think with the previous patch set, it was impossible to set the other 
> channel's gain as Peter pointed out. But at least now that I try, i can 
> change it with alsamixer.
> 
Probably there is some difference with SW? I'm using Debian and
alsa-utils 1.0.21-1.

> > Is this bug with the alsamixer? With amixer I'm able to set positive
> > values but not negative
> >
> > amixer set -D hw:0 'McBSP2 Sidetone Channel 0' 32767
> 
> If you try to give negative values, you need to put '--' before the value.
> 
Thanks, works fine (lazy me).


-- 
Jarkko