[PATCH 0/8]ALSA: ASoc: DaVinci: cleanup

[PATCH 0/8]ALSA: ASoc: DaVinci: cleanup

[Troy Kisky]

Sorry, for the long series to do something so simple, but there were
unresolved objections when I first posted this to the davinci list.

Hopefully, having this split into tiny pieces will make resolving
those differences easier.

[PATCH 1/8] ALSA: ASoC: tlv320aic3x add dsp_a, fix dsp_b

[Troy Kisky]

Add SND_SOC_DAIFMT_DSP_A and fix SND_SOC_DAIFMT_DSP_B
mode. Before dsp_b was really being initialized as dsp_a.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
index b76bcc3..ad19661 100644
--- a/sound/soc/codecs/tlv320aic3x.c
+++ b/sound/soc/codecs/tlv320aic3x.c
@@ -847,6 +847,7 @@ static int aic3x_set_dai_fmt(struct snd_soc_dai *codec_dai,
 	struct snd_soc_codec *codec = codec_dai->codec;
 	struct aic3x_priv *aic3x = codec->private_data;
 	u8 iface_areg, iface_breg;
+	int delay = 0;
 
 	iface_areg = aic3x_read_reg_cache(codec, AIC3X_ASD_INTF_CTRLA) & 0x3f;
 	iface_breg = aic3x_read_reg_cache(codec, AIC3X_ASD_INTF_CTRLB) & 0x3f;
@@ -873,6 +874,8 @@ static int aic3x_set_dai_fmt(struct snd_soc_dai *codec_dai,
 	case (SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF):
 		break;
 	case (SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF):
+		delay = 1;
+	case (SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_IB_NF):
 		iface_breg |= (0x01 << 6);
 		break;
 	case (SND_SOC_DAIFMT_RIGHT_J | SND_SOC_DAIFMT_NB_NF):
@@ -888,6 +891,7 @@ static int aic3x_set_dai_fmt(struct snd_soc_dai *codec_dai,
 	/* set iface */
 	aic3x_write(codec, AIC3X_ASD_INTF_CTRLA, iface_areg);
 	aic3x_write(codec, AIC3X_ASD_INTF_CTRLB, iface_breg);
+	aic3x_write(codec, AIC3X_ASD_INTF_CTRLC, delay);
 
 	return 0;
 }
diff --git a/sound/soc/codecs/tlv320aic3x.h b/sound/soc/codecs/tlv320aic3x.h
index 00a195a..40a6c2a 100644
--- a/sound/soc/codecs/tlv320aic3x.h
+++ b/sound/soc/codecs/tlv320aic3x.h
@@ -35,6 +35,8 @@
 #define AIC3X_ASD_INTF_CTRLA		8
 /* Audio serial data interface control register B */
 #define AIC3X_ASD_INTF_CTRLB		9
+/* Audio serial data interface control register C */
+#define AIC3X_ASD_INTF_CTRLC		10
 /* Audio overflow status and PLL R value programming register */
 #define AIC3X_OVRF_STATUS_AND_PLLR_REG	11
 /* Audio codec digital filter control register */
-- 
1.5.4.3

[PATCH 2/8] ALSA: ASoC: DaVinci: davinvi-evm, make requests explicit

[Troy Kisky]

Add constants with a value of 0 to show more explicitly
what is being requested.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

diff --git a/sound/soc/davinci/davinci-evm.c b/sound/soc/davinci/davinci-evm.c
index 2ce34d4..555ecb2 100644
--- a/sound/soc/davinci/davinci-evm.c
+++ b/sound/soc/davinci/davinci-evm.c
@@ -38,12 +38,14 @@ static int evm_hw_params(struct snd_pcm_substream *substream,
 
 	/* set codec DAI configuration */
 	ret = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_I2S |
-					 SND_SOC_DAIFMT_CBM_CFM);
+					SND_SOC_DAIFMT_CBM_CFM |
+					SND_SOC_DAIFMT_NB_NF);
 	if (ret < 0)
 		return ret;
 
 	/* set cpu DAI configuration */
-	ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_CBM_CFM |
+	ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_I2S |
+					SND_SOC_DAIFMT_CBM_CFM |
 				       SND_SOC_DAIFMT_IB_NF);
 	if (ret < 0)
 		return ret;
-- 
1.5.4.3

[PATCH 3/8] ALSA: ASoC: DaVinci: davinci-i2s add comments to explain polarity

[Troy Kisky]

Document the current polarity choices.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index cf31b3b..ecb5e83 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -235,18 +235,45 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 
 	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
 	case SND_SOC_DAIFMT_IB_NF:
+		/* CLKRP Receive clock polarity,
+		 *	1 - sampled on rising edge of CLKR
+		 *	valid on rising edge
+		 * CLKXP Transmit clock polarity,
+		 *	1 - clocked on falling edge of CLKX
+		 *	valid on rising edge
+		 * FSRP  Receive frame sync pol, 0 - active high
+		 * FSXP  Transmit frame sync pol, 0 - active high
+		 */
 		w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_PCR_REG);
 		MOD_REG_BIT(w, DAVINCI_MCBSP_PCR_CLKXP |
 			       DAVINCI_MCBSP_PCR_CLKRP, 1);
 		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w);
 		break;
 	case SND_SOC_DAIFMT_NB_IF:
+		/* CLKRP Receive clock polarity,
+		 *	0 - sampled on falling edge of CLKR
+		 *	valid on falling edge
+		 * CLKXP Transmit clock polarity,
+		 *	0 - clocked on rising edge of CLKX
+		 *	valid on falling edge
+		 * FSRP  Receive frame sync pol, 1 - active low
+		 * FSXP  Transmit frame sync pol, 1 - active low
+		 */
 		w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_PCR_REG);
 		MOD_REG_BIT(w, DAVINCI_MCBSP_PCR_FSXP |
 			       DAVINCI_MCBSP_PCR_FSRP, 1);
 		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w);
 		break;
 	case SND_SOC_DAIFMT_IB_IF:
+		/* CLKRP Receive clock polarity,
+		 *	1 - sampled on rising edge of CLKR
+		 *	valid on rising edge
+		 * CLKXP Transmit clock polarity,
+		 *	1 - clocked on falling edge of CLKX
+		 *	valid on rising edge
+		 * FSRP  Receive frame sync pol, 1 - active low
+		 * FSXP  Transmit frame sync pol, 1 - active low
+		 */
 		w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_PCR_REG);
 		MOD_REG_BIT(w, DAVINCI_MCBSP_PCR_CLKXP |
 			       DAVINCI_MCBSP_PCR_CLKRP |
@@ -255,6 +282,15 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w);
 		break;
 	case SND_SOC_DAIFMT_NB_NF:
+		/* CLKRP Receive clock polarity,
+		 *	0 - sampled on falling edge of CLKR
+		 *	valid on falling edge
+		 * CLKXP Transmit clock polarity,
+		 *	0 - clocked on rising edge of CLKX
+		 *	valid on falling edge
+		 * FSRP  Receive frame sync pol, 0 - active high
+		 * FSXP  Transmit frame sync pol, 0 - active high
+		 */
 		break;
 	default:
 		return -EINVAL;
-- 
1.5.4.3

[PATCH 4/8] ALSA: ASoC: DaVinci: i2s, evm, pass same value to codec and cpu_dai

[Troy Kisky]

Fix the meaning of SND_SOC_DAIFMT_NB_NF to match that
used in the codec.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

diff --git a/sound/soc/davinci/davinci-evm.c b/sound/soc/davinci/davinci-evm.c
index 555ecb2..a78ff59 100644
--- a/sound/soc/davinci/davinci-evm.c
+++ b/sound/soc/davinci/davinci-evm.c
@@ -28,6 +28,8 @@
 
 #define EVM_CODEC_CLOCK 22579200
 
+#define AUDIO_FORMAT (SND_SOC_DAIFMT_I2S | \
+		SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_NB_NF)
 static int evm_hw_params(struct snd_pcm_substream *substream,
 			 struct snd_pcm_hw_params *params)
 {
@@ -37,16 +39,12 @@ static int evm_hw_params(struct snd_pcm_substream *substream,
 	int ret = 0;
 
 	/* set codec DAI configuration */
-	ret = snd_soc_dai_set_fmt(codec_dai, SND_SOC_DAIFMT_I2S |
-					SND_SOC_DAIFMT_CBM_CFM |
-					SND_SOC_DAIFMT_NB_NF);
+	ret = snd_soc_dai_set_fmt(codec_dai, AUDIO_FORMAT);
 	if (ret < 0)
 		return ret;
 
 	/* set cpu DAI configuration */
-	ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_I2S |
-					SND_SOC_DAIFMT_CBM_CFM |
-				       SND_SOC_DAIFMT_IB_NF);
+	ret = snd_soc_dai_set_fmt(cpu_dai, AUDIO_FORMAT);
 	if (ret < 0)
 		return ret;
 
diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index ecb5e83..8e2cdf2 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -234,7 +234,7 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 	}
 
 	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
-	case SND_SOC_DAIFMT_IB_NF:
+	case SND_SOC_DAIFMT_NB_NF:
 		/* CLKRP Receive clock polarity,
 		 *	1 - sampled on rising edge of CLKR
 		 *	valid on rising edge
@@ -249,7 +249,7 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 			       DAVINCI_MCBSP_PCR_CLKRP, 1);
 		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w);
 		break;
-	case SND_SOC_DAIFMT_NB_IF:
+	case SND_SOC_DAIFMT_IB_IF:
 		/* CLKRP Receive clock polarity,
 		 *	0 - sampled on falling edge of CLKR
 		 *	valid on falling edge
@@ -264,7 +264,7 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 			       DAVINCI_MCBSP_PCR_FSRP, 1);
 		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w);
 		break;
-	case SND_SOC_DAIFMT_IB_IF:
+	case SND_SOC_DAIFMT_NB_IF:
 		/* CLKRP Receive clock polarity,
 		 *	1 - sampled on rising edge of CLKR
 		 *	valid on rising edge
@@ -281,7 +281,7 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 			       DAVINCI_MCBSP_PCR_FSRP, 1);
 		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w);
 		break;
-	case SND_SOC_DAIFMT_NB_NF:
+	case SND_SOC_DAIFMT_IB_NF:
 		/* CLKRP Receive clock polarity,
 		 *	0 - sampled on falling edge of CLKR
 		 *	valid on falling edge
-- 
1.5.4.3

[PATCH 5/8] ALSA: ASoC: DaVinci: davinci-i2s clean up

[Troy Kisky]

Just at little cleanup of davinci_i2s_set_dai_fmt

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index 8e2cdf2..c064348 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -200,36 +200,41 @@ static int davinci_i2s_startup(struct snd_pcm_substream *substream,
 	return 0;
 }
 
+#define DEFAULT_BITPERSAMPLE	16
+
 static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 				   unsigned int fmt)
 {
 	struct davinci_mcbsp_dev *dev = cpu_dai->private_data;
-	u32 w;
+	unsigned int pcr;
+	unsigned int srgr;
+	unsigned int rcr;
+	unsigned int xcr;
+	srgr = DAVINCI_MCBSP_SRGR_FSGM |
+		DAVINCI_MCBSP_SRGR_FPER(DEFAULT_BITPERSAMPLE * 2 - 1) |
+		DAVINCI_MCBSP_SRGR_FWID(DEFAULT_BITPERSAMPLE - 1);
 
 	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 	case SND_SOC_DAIFMT_CBS_CFS:
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG,
-					DAVINCI_MCBSP_PCR_FSXM |
-					DAVINCI_MCBSP_PCR_FSRM |
-					DAVINCI_MCBSP_PCR_CLKXM |
-					DAVINCI_MCBSP_PCR_CLKRM);
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG,
-					DAVINCI_MCBSP_SRGR_FSGM);
+		/* cpu is master */
+		pcr = DAVINCI_MCBSP_PCR_FSXM |
+			DAVINCI_MCBSP_PCR_FSRM |
+			DAVINCI_MCBSP_PCR_CLKXM |
+			DAVINCI_MCBSP_PCR_CLKRM;
 		break;
 	case SND_SOC_DAIFMT_CBM_CFS:
 		/* McBSP CLKR pin is the input for the Sample Rate Generator.
 		 * McBSP FSR and FSX are driven by the Sample Rate Generator. */
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG,
-					DAVINCI_MCBSP_PCR_SCLKME |
-					DAVINCI_MCBSP_PCR_FSXM |
-					DAVINCI_MCBSP_PCR_FSRM);
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG,
-					DAVINCI_MCBSP_SRGR_FSGM);
+		pcr = DAVINCI_MCBSP_PCR_SCLKME |
+			DAVINCI_MCBSP_PCR_FSXM |
+			DAVINCI_MCBSP_PCR_FSRM;
 		break;
 	case SND_SOC_DAIFMT_CBM_CFM:
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, 0);
+		/* codec is master */
+		pcr = 0;
 		break;
 	default:
+		printk(KERN_ERR "%s:bad master\n", __func__);
 		return -EINVAL;
 	}
 
@@ -244,10 +249,7 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		 * FSRP  Receive frame sync pol, 0 - active high
 		 * FSXP  Transmit frame sync pol, 0 - active high
 		 */
-		w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_PCR_REG);
-		MOD_REG_BIT(w, DAVINCI_MCBSP_PCR_CLKXP |
-			       DAVINCI_MCBSP_PCR_CLKRP, 1);
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w);
+		pcr |= (DAVINCI_MCBSP_PCR_CLKXP | DAVINCI_MCBSP_PCR_CLKRP);
 		break;
 	case SND_SOC_DAIFMT_IB_IF:
 		/* CLKRP Receive clock polarity,
@@ -259,10 +261,7 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		 * FSRP  Receive frame sync pol, 1 - active low
 		 * FSXP  Transmit frame sync pol, 1 - active low
 		 */
-		w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_PCR_REG);
-		MOD_REG_BIT(w, DAVINCI_MCBSP_PCR_FSXP |
-			       DAVINCI_MCBSP_PCR_FSRP, 1);
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w);
+		pcr |= (DAVINCI_MCBSP_PCR_FSXP | DAVINCI_MCBSP_PCR_FSRP);
 		break;
 	case SND_SOC_DAIFMT_NB_IF:
 		/* CLKRP Receive clock polarity,
@@ -274,12 +273,8 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		 * FSRP  Receive frame sync pol, 1 - active low
 		 * FSXP  Transmit frame sync pol, 1 - active low
 		 */
-		w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_PCR_REG);
-		MOD_REG_BIT(w, DAVINCI_MCBSP_PCR_CLKXP |
-			       DAVINCI_MCBSP_PCR_CLKRP |
-			       DAVINCI_MCBSP_PCR_FSXP |
-			       DAVINCI_MCBSP_PCR_FSRP, 1);
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, w);
+		pcr |= (DAVINCI_MCBSP_PCR_CLKXP | DAVINCI_MCBSP_PCR_CLKRP |
+			DAVINCI_MCBSP_PCR_FSXP | DAVINCI_MCBSP_PCR_FSRP);
 		break;
 	case SND_SOC_DAIFMT_IB_NF:
 		/* CLKRP Receive clock polarity,
@@ -296,28 +291,24 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		return -EINVAL;
 	}
 
+	rcr = DAVINCI_MCBSP_RCR_RFRLEN1(1);
+	xcr = DAVINCI_MCBSP_XCR_XFIG | DAVINCI_MCBSP_XCR_XFRLEN1(1);
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_RIGHT_J:
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG,
-					DAVINCI_MCBSP_RCR_RFRLEN1(1) |
-					DAVINCI_MCBSP_RCR_RDATDLY(0));
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG,
-					DAVINCI_MCBSP_XCR_XFRLEN1(1) |
-					DAVINCI_MCBSP_XCR_XDATDLY(0) |
-					DAVINCI_MCBSP_XCR_XFIG);
 		break;
 	case SND_SOC_DAIFMT_I2S:
-	default:
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG,
-					DAVINCI_MCBSP_RCR_RFRLEN1(1) |
-					DAVINCI_MCBSP_RCR_RDATDLY(1));
-		davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG,
-					DAVINCI_MCBSP_XCR_XFRLEN1(1) |
-					DAVINCI_MCBSP_XCR_XDATDLY(1) |
-					DAVINCI_MCBSP_XCR_XFIG);
+	case SND_SOC_DAIFMT_DSP_B:
+		rcr |= DAVINCI_MCBSP_RCR_RDATDLY(1);
+		xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1);
 		break;
+	default:
+		printk(KERN_ERR "%s:bad format\n", __func__);
+		return -EINVAL;
 	}
-
+	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr);
+	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, pcr);
+	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG, rcr);
+	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG, xcr);
 	return 0;
 }
 
@@ -343,12 +334,10 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS);
-	w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SRGR_REG);
+	w = DAVINCI_MCBSP_SRGR_FSGM;
 	MOD_REG_BIT(w, DAVINCI_MCBSP_SRGR_FWID(snd_interval_value(i) - 1), 1);
-	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, w);
 
 	i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_FRAME_BITS);
-	w = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SRGR_REG);
 	MOD_REG_BIT(w, DAVINCI_MCBSP_SRGR_FPER(snd_interval_value(i) - 1), 1);
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, w);
 
-- 
1.5.4.3

[PATCH 6/8] ALSA: ASoC: DaVinci: davinci-i2s clean up

[Troy Kisky]

Minor, just move a block of code to make next patch clearer.

Signed-off-by: Troy Kisky <troy.kisky-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index c064348..c425c86 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -238,6 +238,21 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		return -EINVAL;
 	}
 
+	rcr = DAVINCI_MCBSP_RCR_RFRLEN1(1);
+	xcr = DAVINCI_MCBSP_XCR_XFIG | DAVINCI_MCBSP_XCR_XFRLEN1(1);
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_RIGHT_J:
+		break;
+	case SND_SOC_DAIFMT_I2S:
+	case SND_SOC_DAIFMT_DSP_B:
+		rcr |= DAVINCI_MCBSP_RCR_RDATDLY(1);
+		xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1);
+		break;
+	default:
+		printk(KERN_ERR "%s:bad format\n", __func__);
+		return -EINVAL;
+	}
+
 	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
 	case SND_SOC_DAIFMT_NB_NF:
 		/* CLKRP Receive clock polarity,
@@ -290,21 +305,6 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 	default:
 		return -EINVAL;
 	}
-
-	rcr = DAVINCI_MCBSP_RCR_RFRLEN1(1);
-	xcr = DAVINCI_MCBSP_XCR_XFIG | DAVINCI_MCBSP_XCR_XFRLEN1(1);
-	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
-	case SND_SOC_DAIFMT_RIGHT_J:
-		break;
-	case SND_SOC_DAIFMT_I2S:
-	case SND_SOC_DAIFMT_DSP_B:
-		rcr |= DAVINCI_MCBSP_RCR_RDATDLY(1);
-		xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1);
-		break;
-	default:
-		printk(KERN_ERR "%s:bad format\n", __func__);
-		return -EINVAL;
-	}
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr);
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_PCR_REG, pcr);
 	davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG, rcr);
-- 
1.5.4.3

[PATCH 7/8] ALSA: ASoC: DaVinci: document I2S limitations

[Troy Kisky]

DaVinci does not support true I2S or right justified
mode so not all I2S codecs will work with it when the codec is
master. Document this limitation.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
index c425c86..735ec86 100644
--- a/sound/soc/davinci/davinci-i2s.c
+++ b/sound/soc/davinci/davinci-i2s.c
@@ -241,9 +241,26 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 	rcr = DAVINCI_MCBSP_RCR_RFRLEN1(1);
 	xcr = DAVINCI_MCBSP_XCR_XFIG | DAVINCI_MCBSP_XCR_XFRLEN1(1);
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
-	case SND_SOC_DAIFMT_RIGHT_J:
+	case SND_SOC_DAIFMT_DSP_A:
 		break;
 	case SND_SOC_DAIFMT_I2S:
+		/* Davinci doesn't support TRUE I2S, but some codecs will have
+		 * the left and right channels contiguous. This allows
+		 * dsp_b mode to be used with an inverted normal frame clk.
+		 * If your codec is master and does not have contiguous
+		 * channels, then you will have sound on only one channel.
+		 * Try using a different mode, or codec as slave.
+		 *
+		 * The TLV320AIC33 is an example of a codec where this works.
+		 * It has a variable bit clock frequency allowing it to have
+		 * valid data on every bit clock.
+		 *
+		 * The TLV320AIC23 is an example of a codec where this does not
+		 * work. It has a fixed bit clock frequency with progressively
+		 * more empty bit clock slots between channels as the sample
+		 * rate is lowered.
+		 */
+		fmt ^= SND_SOC_DAIFMT_NB_IF;
 	case SND_SOC_DAIFMT_DSP_B:
 		rcr |= DAVINCI_MCBSP_RCR_RDATDLY(1);
 		xcr |= DAVINCI_MCBSP_XCR_XDATDLY(1);
-- 
1.5.4.3

[PATCH 8/8] ALSA: ASoc: DaVinci: davinci-evm use dsp_a mode

[Troy Kisky]

Sense DaVinci does not support true I2S mode and
we don't have to use the hack, use dsp_a mode instead

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

diff --git a/sound/soc/davinci/davinci-evm.c b/sound/soc/davinci/davinci-evm.c
index a78ff59..6c44e4e 100644
--- a/sound/soc/davinci/davinci-evm.c
+++ b/sound/soc/davinci/davinci-evm.c
@@ -28,8 +28,8 @@
 
 #define EVM_CODEC_CLOCK 22579200
 
-#define AUDIO_FORMAT (SND_SOC_DAIFMT_I2S | \
-		SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_NB_NF)
+#define AUDIO_FORMAT (SND_SOC_DAIFMT_DSP_A | \
+		SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_IB_NF)
 static int evm_hw_params(struct snd_pcm_substream *substream,
 			 struct snd_pcm_hw_params *params)
 {
-- 
1.5.4.3

Re: [PATCH 1/8] ALSA: ASoC: tlv320aic3x add dsp_a, fix dsp_b

[Jarkko Nikula]

On Thu, Dec 18, 2008 at 9:36 PM, Troy Kisky
<troy.kisky@boundarydevices.com>wrote:

> Add SND_SOC_DAIFMT_DSP_A and fix SND_SOC_DAIFMT_DSP_B
> mode. Before dsp_b was really being initialized as dsp_a.
>
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>


> @@ -873,6 +874,8 @@ static int aic3x_set_dai_fmt(struct snd_soc_dai
> *codec_dai,
>        case (SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF):
>                break;
>        case (SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF):
> +               delay = 1;
> +       case (SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_IB_NF):
>                iface_breg |= (0x01 << 6);
>                break;
>

Is this correct? I'm looking the WM9713 spec and there DSP_A has the 1-bit
delay.

But I like this idea to add another DSP format.


Jarkko

Re: [PATCH 0/8]ALSA: ASoc: DaVinci: cleanup

[Mark Brown]

On Thu, Dec 18, 2008 at 12:36:38PM -0700, Troy Kisky wrote:

> Sorry, for the long series to do something so simple, but there were
> unresolved objections when I first posted this to the davinci list.

What were the issues people had?

> Hopefully, having this split into tiny pieces will make resolving
> those differences easier.

It certainly makes review easier.

Re: [PATCH 2/8] ALSA: ASoC: DaVinci: davinvi-evm, make requests explicit

[Mark Brown]

On Thu, Dec 18, 2008 at 12:36:40PM -0700, Troy Kisky wrote:
> Add constants with a value of 0 to show more explicitly
> what is being requested.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Applied.

Re: [PATCH 3/8] ALSA: ASoC: DaVinci: davinci-i2s add comments to explain polarity

[Mark Brown]

On Thu, Dec 18, 2008 at 12:36:41PM -0700, Troy Kisky wrote:
> Document the current polarity choices.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Applied.

Re: [PATCH 5/8] ALSA: ASoC: DaVinci: davinci-i2s clean up

[Mark Brown]

On Thu, Dec 18, 2008 at 12:36:43PM -0700, Troy Kisky wrote:
> Just at little cleanup of davinci_i2s_set_dai_fmt
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Applied.

Re: [PATCH 6/8] ALSA: ASoC: DaVinci: davinci-i2s clean up

[Mark Brown]

On Thu, Dec 18, 2008 at 12:36:44PM -0700, Troy Kisky wrote:
> Minor, just move a block of code to make next patch clearer.
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Applied.

Re: [PATCH 4/8] ALSA: ASoC: DaVinci: i2s, evm, pass same value to codec and cpu_dai

[Mark Brown]

On Thu, Dec 18, 2008 at 12:36:42PM -0700, Troy Kisky wrote:
> Fix the meaning of SND_SOC_DAIFMT_NB_NF to match that
> used in the codec.

> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

I'll hold off on this and the other patches that I didn't apply for now
until the disagreements have been resolved.

Re: [PATCH 1/8] ALSA: ASoC: tlv320aic3x add dsp_a, fix dsp_b

[Troy Kisky]

Jarkko Nikula wrote:
> On Thu, Dec 18, 2008 at 9:36 PM, Troy Kisky
> <troy.kisky@boundarydevices.com>wrote:
> 
>> Add SND_SOC_DAIFMT_DSP_A and fix SND_SOC_DAIFMT_DSP_B
>> mode. Before dsp_b was really being initialized as dsp_a.
>>
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>>
> 
> 
>> @@ -873,6 +874,8 @@ static int aic3x_set_dai_fmt(struct snd_soc_dai
>> *codec_dai,
>>        case (SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF):
>>                break;
>>        case (SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF):
>> +               delay = 1;
>> +       case (SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_IB_NF):
>>                iface_breg |= (0x01 << 6);
>>                break;
>>
> 
> Is this correct? I'm looking the WM9713 spec and there DSP_A has the 1-bit
> delay.
> 
> But I like this idea to add another DSP format.
> 
> 
> Jarkko

I think you are correct. Thanks.

But, the AIC23 and SSM2602 would then be backwards too.


Troy

Re: [PATCH 0/8]ALSA: ASoc: DaVinci: cleanup

[Troy Kisky]

Mark Brown wrote:
> On Thu, Dec 18, 2008 at 12:36:38PM -0700, Troy Kisky wrote:
> 
>> Sorry, for the long series to do something so simple, but there were
>> unresolved objections when I first posted this to the davinci list.
> 
> What were the issues people had?
> 
>> Hopefully, having this split into tiny pieces will make resolving
>> those differences easier.
> 
> It certainly makes review easier.
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

The meaning change of SND_SOC_DAIFMT_NB_NF was not liked.
And my comment that I2S mode is not supported on davinci
was not liked. I'll let Naresh explain.

Re: [PATCH 4/8] ALSA: ASoC: DaVinci: i2s, evm, pass same value to codec and cpu_dai

[Troy Kisky]

Mark Brown wrote:
> On Thu, Dec 18, 2008 at 12:36:42PM -0700, Troy Kisky wrote:
>> Fix the meaning of SND_SOC_DAIFMT_NB_NF to match that
>> used in the codec.
> 
>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> 
> I'll hold off on this and the other patches that I didn't apply for now
> until the disagreements have been resolved.
> 

Please drop the remaining patches. I was be sending revised patches
to address my confusion between dsp_a and dsp_b which Jarkko pointed
out.

Thanks

Re: [PATCH 1/8] ALSA: ASoC: tlv320aic3x add dsp_a, fix dsp_b

[Jarkko Nikula]

On Fri, Dec 19, 2008 at 8:04 PM, Troy Kisky
<troy.kisky@boundarydevices.com>wrote:

> Jarkko Nikula wrote:
> > Is this correct? I'm looking the WM9713 spec and there DSP_A has the
> 1-bit
> > delay.
> >
> > But I like this idea to add another DSP format.
> >
> >
> > Jarkko
>
> I think you are correct. Thanks.
>
> But, the AIC23 and SSM2602 would then be backwards too.
>
>
And obviously omap-mcbsp.c too... grr, looks like I forgot to send a fix to
it when fixing aic3x (commit 4b7d283150b35db6e5e10f72606f603ff424c92a). Will
send a fix to omap-mcbsp, aic23 and osk5912 next week.

Yep, seems that SSM2602 should set the submode1 for DSP_B and submode2 for
DSP_A, not vice versa. Worth to check then bf5xx-i2s.c as well.


Jarkko

Re: [PATCH 1/8] ALSA: ASoC: tlv320aic3x add dsp_a, fix dsp_b

[Mark Brown]

On Sat, Dec 20, 2008 at 06:02:33PM +0200, Jarkko Nikula wrote:

> And obviously omap-mcbsp.c too... grr, looks like I forgot to send a fix to
> it when fixing aic3x (commit 4b7d283150b35db6e5e10f72606f603ff424c92a). Will
> send a fix to omap-mcbsp, aic23 and osk5912 next week.

Takashi just said he'll be on holiday from the 23rd so if you could send
a patch before then that'd be best, though if you don't make it then
it's a sufficiently important bug fix to get merged later on.

RE: [alsa-devel] [PATCH 0/8]ALSA: ASoc: DaVinci: cleanup

[Rajashekhara, Sudhakar]

> -----Original Message-----
> From: davinci-linux-open-source-bounces-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org
> [mailto:davinci-linux-open-source-bounces-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org] On Behalf
> Of Troy Kisky
> Sent: Friday, December 19, 2008 11:51 PM
> To: Mark Brown
> Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org; davinci-linux-open-
> source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org; Medisetty, Naresh
> Subject: Re: [alsa-devel] [PATCH 0/8]ALSA: ASoc: DaVinci: cleanup
> 
> Mark Brown wrote:
> > On Thu, Dec 18, 2008 at 12:36:38PM -0700, Troy Kisky wrote:
> >
> >> Sorry, for the long series to do something so simple, but there were
> >> unresolved objections when I first posted this to the davinci list.
> >
> > What were the issues people had?
> >
> >> Hopefully, having this split into tiny pieces will make resolving
> >> those differences easier.
> >
> > It certainly makes review easier.
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >
> 
> The meaning change of SND_SOC_DAIFMT_NB_NF was not liked.
> And my comment that I2S mode is not supported on davinci
> was not liked. I'll let Naresh explain.
> 

The comment regarding I2S mode is absolutely correct, since the davinci cannot support I2S where the codecs (like TLV320AIC23) has fixed bit clock frequency. Documenting this limitation is a very good idea.

Still I find difficulty to agree with Troy regarding the meaning change of SND_SOC_DAIFMT_NB_NF, since the existing meaning is correct w.r.t davinci McBSP. 

> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Re: [PATCH 0/8]ALSA: ASoc: DaVinci: cleanup

[Medisetty, Naresh]

________________________________________
From: Troy Kisky [troy.kisky@boundarydevices.com]
Sent: Friday, December 19, 2008 11:51 PM
To: Mark Brown
Cc: alsa-devel@alsa-project.org; davinci-linux-open-source@linux.davincidsp.com; Medisetty, Naresh
Subject: Re: [alsa-devel] [PATCH 0/8]ALSA: ASoc: DaVinci: cleanup

>
> Mark Brown wrote:
> > On Thu, Dec 18, 2008 at 12:36:38PM -0700, Troy Kisky wrote:
> >
> >> Sorry, for the long series to do something so simple, but there
> >> were unresolved objections when I first posted this to the davinci list.
> >
> > What were the issues people had?
> >
> >> Hopefully, having this split into tiny pieces will make resolving
> >> those differences easier.
> >
> > It certainly makes review easier.
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel@alsa-project.org<mailto:Alsa-devel@alsa-project.org>
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >
>
> The meaning change of SND_SOC_DAIFMT_NB_NF was not liked.
> And my comment that I2S mode is not supported on davinci was not
> liked. I'll let Naresh explain.
>



The comment regarding I2S mode is absolutely correct, since the davinci cannot support I2S where the codecs (like TLV320AIC23) has fixed bit clock frequency. Documenting this limitation is a very good idea.

Still I find difficulty to agree with Troy regarding the meaning change of SND_SOC_DAIFMT_NB_NF, since the existing meaning is correct w.r.t davinci McBSP.



> _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source@linux.davincidsp.com<mailto:Davinci-linux-open-source@linux.davincidsp.com>
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Re: [PATCH 0/8]ALSA: ASoc: DaVinci: cleanup

[David Brownell]

On Sunday 21 December 2008, Rajashekhara, Sudhakar wrote:
> the existing meaning is correct w.r.t davinci McBSP.

Not entirely.  DaVinci doesn't have McBSP; it has ASP.

Having all this code refer to a McBSP -- as found for
example on OMAP chips -- is confusing and incorrect.

- Dave

Re: [PATCH 0/8]ALSA: ASoc: DaVinci: cleanup

[Mark Brown]

On Mon, Dec 22, 2008 at 12:06:05PM +0530, Medisetty, Naresh wrote:

> Still I find difficulty to agree with Troy regarding the meaning change of
> SND_SOC_DAIFMT_NB_NF, since the existing meaning is correct w.r.t davinci
> McBSP.

Could you expand on that a bit - what are you referring to as the
standard here?  This could just be a case of different hardware
manufacturers implementing slightly different things and giving them the
same name.

Re: [PATCH 0/8]ALSA: ASoc: DaVinci: cleanup

[Medisetty, Naresh]

Mark,

> On Mon, Dec 22, 2008 at 12:06:05PM +0530, Medisetty, Naresh wrote:
> 
> > Still I find difficulty to agree with Troy regarding the meaning
> change of
> > SND_SOC_DAIFMT_NB_NF, since the existing meaning is correct w.r.t
> davinci
> > McBSP.
> 
> Could you expand on that a bit - what are you referring to as the 
> standard here?  This could just be a case of different hardware 
> manufacturers implementing slightly different things and giving them 
> the same name.

Yes, Exactly it's just a case of different manufacturers implementing the same thing in different way.

I am referring the davinci ASP default polarities (without inversion) of FS and BCLK as standard.
In davinci ASP Frame sync signals internal to the ASP are active high. 
The Frame sync clock (either FSX or FSR) is XORed with Frame sync polarity bit (FSXP or FSRP).
So if the requirement is for frame sync active low FSXP and FSRP bits in PCR register should be set.

Similarly the BCLK (CLKX and CLKR) is XORed with BCLK polarity bit (CLKXP and CLKRP).
Internally the transmit data driven on rising edge of CLKX and receive data sampled on falling edge of CLKR.
To inverse this CLKXP and CLKRP should be set.

According to this the existing code is proper since it is not setting any polarity bits in _SOC_DAIFMT_NB_NF (Normal bit and Normal Frame )case.

And also in AIC33 codec there is no question of polarities since it is directly configurable through modes (either I2S or DSP).


Regards
Naresh

Re: [PATCH 0/8]ALSA: ASoc: DaVinci: cleanup

[Mark Brown]

On Tue, Dec 23, 2008 at 02:05:33PM +0530, Medisetty, Naresh wrote:
> Mark,

> > Could you expand on that a bit - what are you referring to as the 
> > standard here?  This could just be a case of different hardware 
> > manufacturers implementing slightly different things and giving them 
> > the same name.

> Yes, Exactly it's just a case of different manufacturers implementing the same thing in different way.

The idea in ASoC is to follow a consistent naming scheme within ASoC
rather than implement the APIs in a per-manufacturer fashion.  A given
DAI mode should always produce the same effect in hardware no matter
which device is being configured.  This makes the code much easier to
work with since otherwise you'd not be able to tell from the code if the
devices that are being connected are configured compatibly simply by
looking at the set_dai_fmt() calls.

For most newer SoC CPUs (like the DaVinci) the drivers have to simulate
the various clocking modes by configuring the chip rather than by having
native support for the modes in the chip.

> I am referring the davinci ASP default polarities (without inversion) of FS and BCLK as standard.

Like I say, the DaVinci chip defaults don't matter here - what matters
is the mode the DAI is supposed to be operating in.

> In davinci ASP Frame sync signals internal to the ASP are active high. 
> The Frame sync clock (either FSX or FSR) is XORed with Frame sync polarity bit (FSXP or FSRP).
> So if the requirement is for frame sync active low FSXP and FSRP bits in PCR register should be set.

Troy left the frame clock settings alone so I'll assume you've no
concerns there?

> Similarly the BCLK (CLKX and CLKR) is XORed with BCLK polarity bit (CLKXP and CLKRP).
> Internally the transmit data driven on rising edge of CLKX and receive data sampled on falling edge of CLKR.
> To inverse this CLKXP and CLKRP should be set.

> According to this the existing code is proper since it is not setting any polarity bits in _SOC_DAIFMT_NB_NF (Normal bit and Normal Frame )case.

The DaVinci driver is configuring the clocks to match DSP mode (when
using I2S mode it flips the inversion bits in the format before using
them).  For DSP mode data is available on the first rising edge of BCLK
so sampling should be done on the rising edge.  This means that the data
has to be driven out on the falling edge to ensure that it is ready to
be sampled on the rising edge.  Troy's changes implement this so they
look correct to me.

> And also in AIC33 codec there is no question of polarities since it is directly configurable through modes (either I2S or DSP).

The inversions apply to the standard clocking for a given mode.  For
example, with I2S LRCLK clock inversion is essentially equivalent to
swapping left and right channels.  This facility is implemented by a
reasonable number of codecs and made available through the API since
it improves interoperability.  Some CPUs (especially those with hand
configured ports rather than native support for the clocking modes)
have trouble doing some of the standard modes but can deal with inverted
versions of one or both of the clocks.

[BTW, your MUA appears to be generating lines ~90-100 characters long -
it'd help if it were to go for something less than 80 characters since
it makes your messages easier to read and reply to with standard mail
clients.]

Re: [PATCH 0/8]ALSA: ASoc: DaVinci: cleanup

[Medisetty, Naresh]

Mark,

> 
> > > Could you expand on that a bit - what are you referring to as the 
> > > standard here?  This could just be a case of different hardware 
> > > manufacturers implementing slightly different things and giving
> them
> > > the same name.
> 
> > Yes, Exactly it's just a case of different manufacturers 
> > implementing
> the same thing in different way.
> 
> The idea in ASoC is to follow a consistent naming scheme within ASoC 
> rather than implement the APIs in a per-manufacturer fashion.  A given 
> DAI mode should always produce the same effect in hardware no matter 
> which device is being configured.  This makes the code much easier to 
> work with since otherwise you'd not be able to tell from the code if 
> the devices that are being connected are configured compatibly simply 
> by looking at the set_dai_fmt() calls.
> 

>From this I came to conclusion in ASoC Normal FS means active high

and Normal BCLK means receive data sampled on rising edge and transmit 

data driven on falling edge. Correct me if I am wrong here.

> For most newer SoC CPUs (like the DaVinci) the drivers have to 
> simulate the various clocking modes by configuring the chip rather 
> than by having native support for the modes in the chip.
> 
> > I am referring the davinci ASP default polarities (without 
> > inversion)
> of FS and BCLK as standard.
> 
> Like I say, the DaVinci chip defaults don't matter here - what matters 
> is the mode the DAI is supposed to be operating in.
> 
> > In davinci ASP Frame sync signals internal to the ASP are active
> high.
> > The Frame sync clock (either FSX or FSR) is XORed with Frame sync
> polarity bit (FSXP or FSRP).
> > So if the requirement is for frame sync active low FSXP and FSRP 
> > bits
> in PCR register should be set.
> 
> Troy left the frame clock settings alone so I'll assume you've no 
> concerns there?

Yes, No concerns here

> 
> > Similarly the BCLK (CLKX and CLKR) is XORed with BCLK polarity bit
> (CLKXP and CLKRP).
> > Internally the transmit data driven on rising edge of CLKX and
> receive data sampled on falling edge of CLKR.
> > To inverse this CLKXP and CLKRP should be set.
> 
> > According to this the existing code is proper since it is not 
> > setting
> any polarity bits in _SOC_DAIFMT_NB_NF (Normal bit and Normal Frame 
> )case.
> 
> The DaVinci driver is configuring the clocks to match DSP mode (when 
> using I2S mode it flips the inversion bits in the format before using 
> them).  For DSP mode data is available on the first rising edge of 
> BCLK so sampling should be done on the rising edge.  This means that 
> the data has to be driven out on the falling edge to ensure that it is 
> ready to be sampled on the rising edge.  Troy's changes implement this 
> so they look correct to me.
> 

Yes, its correct according to ASoC polarity standards

> > And also in AIC33 codec there is no question of polarities since it
> is directly configurable through modes (either I2S or DSP).
> 
> The inversions apply to the standard clocking for a given mode.  For 
> example, with I2S LRCLK clock inversion is essentially equivalent to 
> swapping left and right channels.  This facility is implemented by a 
> reasonable number of codecs and made available through the API since 
> it improves interoperability.  Some CPUs (especially those with hand 
> configured ports rather than native support for the clocking modes) 
> have trouble doing some of the standard modes but can deal with 
> inverted versions of one or both of the clocks.

Thanks  

> 
> [BTW, your MUA appears to be generating lines ~90-100 characters long 
> - it'd help if it were to go for something less than 80 characters 
> since it makes your messages easier to read and reply to with standard 
> mail clients.]

Sorry, I will take care of this 


Regards
Naresh

Re: [PATCH 0/8]ALSA: ASoc: DaVinci: cleanup

[Mark Brown]

On Wed, Dec 24, 2008 at 12:06:56PM +0530, Medisetty, Naresh wrote:
> Mark,

> > DAI mode should always produce the same effect in hardware no matter 
> > which device is being configured.  This makes the code much easier to 

> From this I came to conclusion in ASoC Normal FS means active high
> and Normal BCLK means receive data sampled on rising edge and transmit 
> data driven on falling edge. Correct me if I am wrong here.

No.  It means whatever is standard for the mode (DSP, I2S, left
justified and so on) that the DAI is operating in.  For DSP mode the
above is accurate but the others differ.  The reason why the same
case statement is used for all modes in the DaVinci mode is that when
I2S mode is selected the user provided bit/frame inversion options are
changed before they are used.