[PATCH 0/7] ASoC:davinci-mcasp: bc polarity fix and proper format support

[PATCH 0/7] ASoC:davinci-mcasp: bc polarity fix and proper format support

[Peter Ujfalusi]

Hi,

The bit clock polarity has been configured incorrectly in the McASP driver for a
long time. IB_NF, NB_IF and IB_IF was not correct on the receive side since they
were selecting the same edge as we used for the tx.

The driver only had support for DSP_B mode (and probably AC97, but I can not
test that). All other formats were broken (DSP_A, I2S, LEFT_J, etc).
With this series the following formats will be supported by the McASP driver:
- DSP_B
- AC97
- DSP_A (new)
- I2S (new)
- LEFT_J (new)

Non supported modes will return error from now on.

Regards,
Peter
---
Peter Ujfalusi (7):
  ASoC: davinci-mcasp: Fix bit clock polarity settings
  ASoC: davinci-mcasp: Format data delay configuration enhancement
  ASoC: davinci-mcasp: Support for DSP_A format
  ASoC: davinci-mcasp: Move the FS polarity change out from the switch
    case
  ASoC: davinci-mcasp: Add support for I2S format
  ASoC: davinci-mcasp: Support for LEFT_J format
  ASoC: davinci-mcasp: Remove excess empty lines from
    davinci_mcasp_set_dai_fmt()

 sound/soc/davinci/davinci-mcasp.c | 76 +++++++++++++++++++++++++--------------
 1 file changed, 50 insertions(+), 26 deletions(-)

-- 
1.9.1

[PATCH 1/7] ASoC: davinci-mcasp: Fix bit clock polarity settings

[Peter Ujfalusi]

IB_NF, NB_IF and IB_IF configured the bc polarity incorrectly. The receive
polarity was set to the same edge as the TX in these cases.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index aae7d69af699..95b38f74e2af 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -331,7 +331,7 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_ACLKXCTL_REG, ACLKXPOL);
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXPOL);
 
-		mcasp_set_bits(mcasp, DAVINCI_MCASP_ACLKRCTL_REG, ACLKRPOL);
+		mcasp_clr_bits(mcasp, DAVINCI_MCASP_ACLKRCTL_REG, ACLKRPOL);
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRPOL);
 		break;
 
@@ -339,7 +339,7 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_ACLKXCTL_REG, ACLKXPOL);
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXPOL);
 
-		mcasp_clr_bits(mcasp, DAVINCI_MCASP_ACLKRCTL_REG, ACLKRPOL);
+		mcasp_set_bits(mcasp, DAVINCI_MCASP_ACLKRCTL_REG, ACLKRPOL);
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRPOL);
 		break;
 
@@ -347,7 +347,7 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_ACLKXCTL_REG, ACLKXPOL);
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXPOL);
 
-		mcasp_set_bits(mcasp, DAVINCI_MCASP_ACLKRCTL_REG, ACLKRPOL);
+		mcasp_clr_bits(mcasp, DAVINCI_MCASP_ACLKRCTL_REG, ACLKRPOL);
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRPOL);
 		break;
 
-- 
1.9.1

Re: [PATCH 1/7] ASoC: davinci-mcasp: Fix bit clock polarity settings

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 368 bytes --]

On Fri, Apr 04, 2014 at 02:31:41PM +0300, Peter Ujfalusi wrote:
> IB_NF, NB_IF and IB_IF configured the bc polarity incorrectly. The receive
> polarity was set to the same edge as the TX in these cases.

Applied, thanks.  I'll apply the rest after -rc1 comes out since there's
an interdependency between this fix and the rest and I don't want to
rebase at the minute.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 2/7] ASoC: davinci-mcasp: Format data delay configuration enhancement

[Peter Ujfalusi]

Use intermediate variable for the data delay needed for the specific format
and write the register after the format configuration at once.
This will help to control the number of lines as support for more formats
going to be added.
Also fixes a case when we switch between two formats with different delay
requirements.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index 95b38f74e2af..d9ea709cb6d0 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -264,6 +264,7 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 {
 	struct davinci_mcasp *mcasp = snd_soc_dai_get_drvdata(cpu_dai);
 	int ret = 0;
+	u32 data_delay;
 
 	pm_runtime_get_sync(mcasp->dev);
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
@@ -271,18 +272,25 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 	case SND_SOC_DAIFMT_AC97:
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXDUR);
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRDUR);
+
+		/* No delay after FS */
+		data_delay = 0;
 		break;
 	default:
 		/* configure a full-word SYNC pulse (LRCLK) */
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXDUR);
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRDUR);
 
-		/* make 1st data bit occur one ACLK cycle after the frame sync */
-		mcasp_set_bits(mcasp, DAVINCI_MCASP_TXFMT_REG, FSXDLY(1));
-		mcasp_set_bits(mcasp, DAVINCI_MCASP_RXFMT_REG, FSRDLY(1));
+		/* 1st data bit occur one ACLK cycle after the frame sync */
+		data_delay = 1;
 		break;
 	}
 
+	mcasp_mod_bits(mcasp, DAVINCI_MCASP_TXFMT_REG, FSXDLY(data_delay),
+		       FSXDLY(3));
+	mcasp_mod_bits(mcasp, DAVINCI_MCASP_RXFMT_REG, FSRDLY(data_delay),
+		       FSRDLY(3));
+
 	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 	case SND_SOC_DAIFMT_CBS_CFS:
 		/* codec is clock and frame slave */
-- 
1.9.1

[PATCH 3/7] ASoC: davinci-mcasp: Support for DSP_A format

[Peter Ujfalusi]

DSP_A is like DSP_B mode but with one bit delay after the FS.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index d9ea709cb6d0..7076dba66031 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -268,6 +268,13 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 
 	pm_runtime_get_sync(mcasp->dev);
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_DSP_A:
+		mcasp_clr_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXDUR);
+		mcasp_clr_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRDUR);
+
+		/* 1st data bit occur one ACLK cycle after the frame sync */
+		data_delay = 1;
+		break;
 	case SND_SOC_DAIFMT_DSP_B:
 	case SND_SOC_DAIFMT_AC97:
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXDUR);
-- 
1.9.1

[PATCH 4/7] ASoC: davinci-mcasp: Move the FS polarity change out from the switch case

[Peter Ujfalusi]

FS polarity can be either rising or falling edge in McASP. Instead of
accessing the registers in every switch/case set a flag and write the
registers after the switch for the invert configuration.
This change will help when adding support for more formats also.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index 7076dba66031..3f67e374829e 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -265,6 +265,7 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 	struct davinci_mcasp *mcasp = snd_soc_dai_get_drvdata(cpu_dai);
 	int ret = 0;
 	u32 data_delay;
+	bool fs_pol_rising;
 
 	pm_runtime_get_sync(mcasp->dev);
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
@@ -344,39 +345,39 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
 	case SND_SOC_DAIFMT_IB_NF:
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_ACLKXCTL_REG, ACLKXPOL);
-		mcasp_clr_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXPOL);
-
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_ACLKRCTL_REG, ACLKRPOL);
-		mcasp_clr_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRPOL);
+		fs_pol_rising = true;
 		break;
 
 	case SND_SOC_DAIFMT_NB_IF:
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_ACLKXCTL_REG, ACLKXPOL);
-		mcasp_set_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXPOL);
-
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_ACLKRCTL_REG, ACLKRPOL);
-		mcasp_set_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRPOL);
+		fs_pol_rising = false;
 		break;
 
 	case SND_SOC_DAIFMT_IB_IF:
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_ACLKXCTL_REG, ACLKXPOL);
-		mcasp_set_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXPOL);
-
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_ACLKRCTL_REG, ACLKRPOL);
-		mcasp_set_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRPOL);
+		fs_pol_rising = false;
 		break;
 
 	case SND_SOC_DAIFMT_NB_NF:
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_ACLKXCTL_REG, ACLKXPOL);
-		mcasp_clr_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXPOL);
-
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_ACLKRCTL_REG, ACLKRPOL);
-		mcasp_clr_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRPOL);
+		fs_pol_rising = true;
 		break;
 
 	default:
 		ret = -EINVAL;
-		break;
+		goto out;
+	}
+
+	if (fs_pol_rising) {
+		mcasp_clr_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXPOL);
+		mcasp_clr_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRPOL);
+	} else {
+		mcasp_set_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXPOL);
+		mcasp_set_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRPOL);
 	}
 out:
 	pm_runtime_put_sync(mcasp->dev);
-- 
1.9.1

[PATCH 5/7] ASoC: davinci-mcasp: Add support for I2S format

[Peter Ujfalusi]

The FS needs to be inverted in McASP compared to other supported formats.
Use a flag to indicate if the FS needs to be inverted.
At the same time fail when non supported format is asked since the default
case was anyways configuring McASP to a not valid format.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index 3f67e374829e..af6cca3e61e4 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -266,6 +266,7 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 	int ret = 0;
 	u32 data_delay;
 	bool fs_pol_rising;
+	bool inv_fs = false;
 
 	pm_runtime_get_sync(mcasp->dev);
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
@@ -284,14 +285,19 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		/* No delay after FS */
 		data_delay = 0;
 		break;
-	default:
+	case SND_SOC_DAIFMT_I2S:
 		/* configure a full-word SYNC pulse (LRCLK) */
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXDUR);
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRDUR);
 
 		/* 1st data bit occur one ACLK cycle after the frame sync */
 		data_delay = 1;
+		/* FS need to be inverted */
+		inv_fs = true;
 		break;
+	default:
+		ret = -EINVAL;
+		goto out;
 	}
 
 	mcasp_mod_bits(mcasp, DAVINCI_MCASP_TXFMT_REG, FSXDLY(data_delay),
@@ -372,6 +378,9 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		goto out;
 	}
 
+	if (inv_fs)
+		fs_pol_rising = !fs_pol_rising;
+
 	if (fs_pol_rising) {
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXPOL);
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRPOL);
-- 
1.9.1

[PATCH 6/7] ASoC: davinci-mcasp: Support for LEFT_J format

[Peter Ujfalusi]

Configuration for LEFT_J format.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index af6cca3e61e4..da9533b8e2dd 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -295,6 +295,13 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		/* FS need to be inverted */
 		inv_fs = true;
 		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		/* configure a full-word SYNC pulse (LRCLK) */
+		mcasp_set_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXDUR);
+		mcasp_set_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRDUR);
+		/* No delay after FS */
+		data_delay = 0;
+		break;
 	default:
 		ret = -EINVAL;
 		goto out;
-- 
1.9.1

[PATCH 7/7] ASoC: davinci-mcasp: Remove excess empty lines from davinci_mcasp_set_dai_fmt()

[Peter Ujfalusi]

To make the code look uniform.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/davinci/davinci-mcasp.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/sound/soc/davinci/davinci-mcasp.c b/sound/soc/davinci/davinci-mcasp.c
index da9533b8e2dd..ce0e55a4304c 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -273,7 +273,6 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 	case SND_SOC_DAIFMT_DSP_A:
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXDUR);
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRDUR);
-
 		/* 1st data bit occur one ACLK cycle after the frame sync */
 		data_delay = 1;
 		break;
@@ -281,7 +280,6 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 	case SND_SOC_DAIFMT_AC97:
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXDUR);
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRDUR);
-
 		/* No delay after FS */
 		data_delay = 0;
 		break;
@@ -289,7 +287,6 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		/* configure a full-word SYNC pulse (LRCLK) */
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_TXFMCTL_REG, FSXDUR);
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_RXFMCTL_REG, FSRDUR);
-
 		/* 1st data bit occur one ACLK cycle after the frame sync */
 		data_delay = 1;
 		/* FS need to be inverted */
@@ -349,7 +346,6 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 			       ACLKX | AHCLKX | AFSX | ACLKR | AHCLKR | AFSR);
 		mcasp->bclk_master = 0;
 		break;
-
 	default:
 		ret = -EINVAL;
 		goto out;
@@ -361,25 +357,21 @@ static int davinci_mcasp_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_ACLKRCTL_REG, ACLKRPOL);
 		fs_pol_rising = true;
 		break;
-
 	case SND_SOC_DAIFMT_NB_IF:
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_ACLKXCTL_REG, ACLKXPOL);
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_ACLKRCTL_REG, ACLKRPOL);
 		fs_pol_rising = false;
 		break;
-
 	case SND_SOC_DAIFMT_IB_IF:
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_ACLKXCTL_REG, ACLKXPOL);
 		mcasp_clr_bits(mcasp, DAVINCI_MCASP_ACLKRCTL_REG, ACLKRPOL);
 		fs_pol_rising = false;
 		break;
-
 	case SND_SOC_DAIFMT_NB_NF:
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_ACLKXCTL_REG, ACLKXPOL);
 		mcasp_set_bits(mcasp, DAVINCI_MCASP_ACLKRCTL_REG, ACLKRPOL);
 		fs_pol_rising = true;
 		break;
-
 	default:
 		ret = -EINVAL;
 		goto out;
-- 
1.9.1

Re: [PATCH 0/7] ASoC:davinci-mcasp: bc polarity fix and proper format support

[Daniel Mack]

Hi Peter,

On 04/04/2014 01:31 PM, Peter Ujfalusi wrote:
> The bit clock polarity has been configured incorrectly in the McASP driver for a
> long time. IB_NF, NB_IF and IB_IF was not correct on the receive side since they
> were selecting the same edge as we used for the tx.
> 
> The driver only had support for DSP_B mode (and probably AC97, but I can not
> test that). All other formats were broken (DSP_A, I2S, LEFT_J, etc).

Well, we're using this driver in I2S mode for a number of devices since
a while, so the above statement is not entirely true :)

The cleanups look sane, though. I can test them on AM33xx based hardware
early next week. If you don't want to hold them off until then, no
problem. I can also send fixups in case I spot a regression.

Out of interest: which hardware and which dai format are you testing
this with?


Thanks,
Daniel

Re: [PATCH 0/7] ASoC:davinci-mcasp: bc polarity fix and proper format support

[Peter Ujfalusi]

Hi Daniel,

On 04/04/2014 04:24 PM, Daniel Mack wrote:
> Hi Peter,
> 
> On 04/04/2014 01:31 PM, Peter Ujfalusi wrote:
>> The bit clock polarity has been configured incorrectly in the McASP driver for a
>> long time. IB_NF, NB_IF and IB_IF was not correct on the receive side since they
>> were selecting the same edge as we used for the tx.
>>
>> The driver only had support for DSP_B mode (and probably AC97, but I can not
>> test that). All other formats were broken (DSP_A, I2S, LEFT_J, etc).
> 
> Well, we're using this driver in I2S mode for a number of devices since
> a while, so the above statement is not entirely true :)

True, you could select I2S and ask for inverted frame sync with the old code.
However if you ask for DSP_A, LEFT_J, etc it was doing the wrong thing.

> The cleanups look sane, though. I can test them on AM33xx based hardware
> early next week. If you don't want to hold them off until then, no
> problem. I can also send fixups in case I spot a regression.

That would be great if you could also test these. It seams you have quite good
array of codecs available.

> Out of interest: which hardware and which dai format are you testing
> this with?

I have one AM335x board, one AM437x and one DRA7 where I can test right now. I
just go one OMAP-l138 board which I need to set up. It is good for legacy (non
DT boot) debugging.

> 
> 
> Thanks,
> Daniel
> 


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

Re: [PATCH 0/7] ASoC:davinci-mcasp: bc polarity fix and proper format support

[Daniel Mack]

Hi Peter,

On 04/04/2014 06:38 PM, Peter Ujfalusi wrote:
> On 04/04/2014 04:24 PM, Daniel Mack wrote:

>> The cleanups look sane, though. I can test them on AM33xx based hardware
>> early next week. If you don't want to hold them off until then, no
>> problem. I can also send fixups in case I spot a regression.
> 
> That would be great if you could also test these. It seams you have quite good
> array of codecs available.

I tested this series with a CS4271 codec in I2S mode, and things still
work as expected, except for the fact that the LRCLK is now inverted in
comparison to the old state.

After some debugging, however, I have confidence that the behaviour with
the patches applied is in fact correct, we just need to flip the bits in
our ASoC machine driver, which is no big deal.


Thanks for the cleanups again!
Daniel

Re: [PATCH 0/7] ASoC:davinci-mcasp: bc polarity fix and proper format support

[Peter Ujfalusi]

Hi Daniel,

On 04/09/2014 03:12 PM, Daniel Mack wrote:
> Hi Peter,
> 
> On 04/04/2014 06:38 PM, Peter Ujfalusi wrote:
>> On 04/04/2014 04:24 PM, Daniel Mack wrote:
> 
>>> The cleanups look sane, though. I can test them on AM33xx based hardware
>>> early next week. If you don't want to hold them off until then, no
>>> problem. I can also send fixups in case I spot a regression.
>>
>> That would be great if you could also test these. It seams you have quite good
>> array of codecs available.
> 
> I tested this series with a CS4271 codec in I2S mode, and things still
> work as expected, except for the fact that the LRCLK is now inverted in
> comparison to the old state.

Yes, this is expected. I think you were asking for NB_IF with the old code,
right? The old code was - wrongly IMHO - configured the I2S mode's FS in
inverted mode: NB_NF would end up to send the left channel when the FS is
high. This is not correct. NB_NF means in I2S is that left channel is when FS
is low, right channel is when FS is high.
Most of the daVinci device used DSP_B, which was correct in terms of FS.
The NB_IF with cs4271 was fine since the codec driver did not checked for
invert it only cares about the protocol. Page 20 of the cs4271 shows the I2S.
With this series we can use the correct DAIFMT_I2S + NB_NF with standard
compliant codecs.

> After some debugging, however, I have confidence that the behaviour with
> the patches applied is in fact correct, we just need to flip the bits in
> our ASoC machine driver, which is no big deal.
> 
> 
> Thanks for the cleanups again!

Thanks for testing it!

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

Re: [PATCH 0/7] ASoC:davinci-mcasp: bc polarity fix and proper format support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 328 bytes --]

On Fri, Apr 04, 2014 at 02:31:40PM +0300, Peter Ujfalusi wrote:
> Hi,
> 
> The bit clock polarity has been configured incorrectly in the McASP driver for a
> long time. IB_NF, NB_IF and IB_IF was not correct on the receive side since they
> were selecting the same edge as we used for the tx.

Applied the rest, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]