[PATCH 0/2] ASoC: DSP mode corrections for omap-mcbsp

[PATCH 0/2] ASoC: DSP mode corrections for omap-mcbsp

[Peter Ujfalusi]

Hello,

As a follow-up for the before easter mail thread on the omap DSP_A/_B mode
support...

This series should fix the current DSP_B mode and also introduces support for
DSP_A mode in omap-mcbsp.

Since omap's mcbsp module is quite flexible, one who is writing a board file for
the omap platform has to check both the modes (DSP_A/_B) and the clock polarity
settings against the codec requirements and pick the correct pair.

For example in osk5912 board file:
To use DSP_B mode (as it is now, no changes needed):
        snd_soc_dai_set_fmt(codec_dai,
                SND_SOC_DAIFMT_DSP_B |
                SND_SOC_DAIFMT_NB_IF |
                SND_SOC_DAIFMT_CBM_CFM);

To use the DSP_A mode for the same board, codec:
        snd_soc_dai_set_fmt(codec_dai,
                SND_SOC_DAIFMT_DSP_A |
                SND_SOC_DAIFMT_IB_IF |
                SND_SOC_DAIFMT_CBM_CFM);


---
Peter Ujfalusi (2):
  ASoC: omap-mcbsp: Correct the DSP_B mode
  ASoC: omap-mcbsp: Add DSP_A support

 sound/soc/omap/omap-mcbsp.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

[PATCH 1/2] ASoC: omap-mcbsp: Correct the DSP_B mode

[Peter Ujfalusi]

Use correct DSP_B mode configuration for omap-mcbsp.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 sound/soc/omap/omap-mcbsp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
index 9c09b94..4440bab 100644
--- a/sound/soc/omap/omap-mcbsp.c
+++ b/sound/soc/omap/omap-mcbsp.c
@@ -283,7 +283,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream,
 		break;
 	case SND_SOC_DAIFMT_DSP_B:
 		regs->srgr2	|= FPER(wlen * channels - 1);
-		regs->srgr1	|= FWID(wlen * channels - 2);
+		regs->srgr1	|= FWID(0);
 		break;
 	}
 
-- 
1.6.2.2

[PATCH 2/2] ASoC: omap-mcbsp: Add DSP_A support

[Peter Ujfalusi]

Add support for DSP_A in omap-mcbsp.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 sound/soc/omap/omap-mcbsp.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
index 4440bab..1576e70 100644
--- a/sound/soc/omap/omap-mcbsp.c
+++ b/sound/soc/omap/omap-mcbsp.c
@@ -281,6 +281,10 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream,
 		regs->srgr2	|= FPER(wlen * 2 - 1);
 		regs->srgr1	|= FWID(wlen - 1);
 		break;
+	case SND_SOC_DAIFMT_DSP_A:
+		regs->srgr2	|= FPER(wlen * channels - 1);
+		regs->srgr1	|= FWID(wlen * channels - 2);
+		break;
 	case SND_SOC_DAIFMT_DSP_B:
 		regs->srgr2	|= FPER(wlen * channels - 1);
 		regs->srgr1	|= FWID(0);
@@ -324,6 +328,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		regs->rcr2	|= RDATDLY(1);
 		regs->xcr2	|= XDATDLY(1);
 		break;
+	case SND_SOC_DAIFMT_DSP_A:
 	case SND_SOC_DAIFMT_DSP_B:
 		/* 0-bit data delay */
 		regs->rcr2      |= RDATDLY(0);
-- 
1.6.2.2

Re: [PATCH 1/2] ASoC: omap-mcbsp: Correct the DSP_B mode

[Jarkko Nikula]

On Tue, 14 Apr 2009 09:45:30 +0200
"Ujfalusi Peter (Nokia-D/Tampere)" <peter.ujfalusi@nokia.com> wrote:

> Use correct DSP_B mode configuration for omap-mcbsp.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
> ---
>  sound/soc/omap/omap-mcbsp.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
> index 9c09b94..4440bab 100644
> --- a/sound/soc/omap/omap-mcbsp.c
> +++ b/sound/soc/omap/omap-mcbsp.c
> @@ -283,7 +283,7 @@ static int omap_mcbsp_dai_hw_params(struct
> snd_pcm_substream *substream, break;
>  	case SND_SOC_DAIFMT_DSP_B:
>  		regs->srgr2	|= FPER(wlen * channels - 1);
> -		regs->srgr1	|= FWID(wlen * channels - 2);
> +		regs->srgr1	|= FWID(0);
>  		break;
>  	}

Grr, I have some practical problems with my Beagle refusing to mount my
SD card root but I give a test when I get it working.

But I fear this change makes it into DSP_B with inverted FS polarity.
Register srgr1 defines the FS length and it's active low. I think this
will be correct if you switch the polarity in
omap_mcbsp_dai_set_dai_fmt.

At least defining pulse witdth with FWID(0) is more clear than FWID
(wlen * channels - 2) :-)


Jarkko

Re: [PATCH 1/2] ASoC: omap-mcbsp: Correct the DSP_B mode

[Peter Ujfalusi]

On Tuesday 14 April 2009 11:20:16 Nikula Jarkko (Nokia-D/Helsinki) wrote:
> On Tue, 14 Apr 2009 09:45:30 +0200
>
> "Ujfalusi Peter (Nokia-D/Tampere)" <peter.ujfalusi@nokia.com> wrote:
> > Use correct DSP_B mode configuration for omap-mcbsp.
> >
> > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
> > ---
> >  sound/soc/omap/omap-mcbsp.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
> > index 9c09b94..4440bab 100644
> > --- a/sound/soc/omap/omap-mcbsp.c
> > +++ b/sound/soc/omap/omap-mcbsp.c
> > @@ -283,7 +283,7 @@ static int omap_mcbsp_dai_hw_params(struct
> > snd_pcm_substream *substream, break;
> >  	case SND_SOC_DAIFMT_DSP_B:
> >  		regs->srgr2	|= FPER(wlen * channels - 1);
> > -		regs->srgr1	|= FWID(wlen * channels - 2);
> > +		regs->srgr1	|= FWID(0);
> >  		break;
> >  	}
>
> Grr, I have some practical problems with my Beagle refusing to mount my
> SD card root but I give a test when I get it working.

If you want to use the TWL codec to verify the DSP mode, than you will need to 
have the 4 channel support also. I'm planning to resend those as soon as this 
DSP_A/_B mode is settled.

>
> But I fear this change makes it into DSP_B with inverted FS polarity.
> Register srgr1 defines the FS length and it's active low. I think this
> will be correct if you switch the polarity in
> omap_mcbsp_dai_set_dai_fmt.

As I written in the introduction mail, one has to verify and pick the correct 
pair of DSP_A/_B mode and FS/clock polarity in order to have correct config...
I think it is acceptable to assume, that whoever writes such a board file 
should have at least this level of understanding of the FS/bitclock and how 
the data is driven and how the data is sampled.

Also I have provided sample configurations for the osk5912 board file - which 
is the only one at the moment using DSP mode on omap platform.

>
> At least defining pulse witdth with FWID(0) is more clear than FWID
> (wlen * channels - 2) :-)

I think this is the only way to implement the DSP_B mode correctly in omap-
mcbsp. Again, you need to pick the correct FS/clock polarity, but that is kind 
of simple.

>
>
> Jarkko

-- 
Péter

Re: [PATCH 1/2] ASoC: omap-mcbsp: Correct the DSP_B mode

[Jarkko Nikula]

On Tue, 14 Apr 2009 10:40:16 +0200
"Ujfalusi Peter (Nokia-D/Tampere)" <peter.ujfalusi@nokia.com> wrote:

> > But I fear this change makes it into DSP_B with inverted FS
> > polarity. Register srgr1 defines the FS length and it's active low.
> > I think this will be correct if you switch the polarity in
> > omap_mcbsp_dai_set_dai_fmt.
> 
> As I written in the introduction mail, one has to verify and pick the
> correct pair of DSP_A/_B mode and FS/clock polarity in order to have
> correct config... I think it is acceptable to assume, that whoever
> writes such a board file should have at least this level of
> understanding of the FS/bitclock and how the data is driven and how
> the data is sampled.
> 
The pairs must mach. It's clearly a bug if another end is have to set
different polarity than another. Those polarity defines are with
respect to the mode, not e.g. what's the McBSP default.


Jarkko

Re: [PATCH 1/2] ASoC: omap-mcbsp: Correct the DSP_B mode

[Mark Brown]

On Tue, Apr 14, 2009 at 12:02:21PM +0300, Jarkko Nikula wrote:

> The pairs must mach. It's clearly a bug if another end is have to set
> different polarity than another. Those polarity defines are with
> respect to the mode, not e.g. what's the McBSP default.

Indeed.  However, the osk5912 board file that Peter pointed at as not
needing any modifications for DSP_B already uses inverted frame clock
for both CODEC and CPU.  In that case either both are wrong or it's just
that the driver is limited in what it supports which is not a problem.

Re: [PATCH 1/2] ASoC: omap-mcbsp: Correct the DSP_B mode

[Peter Ujfalusi]

On Tuesday 14 April 2009 12:31:33 ext Mark Brown wrote:
> On Tue, Apr 14, 2009 at 12:02:21PM +0300, Jarkko Nikula wrote:
> > The pairs must mach. It's clearly a bug if another end is have to set
> > different polarity than another. Those polarity defines are with
> > respect to the mode, not e.g. what's the McBSP default.
>
> Indeed.  However, the osk5912 board file that Peter pointed at as not
> needing any modifications for DSP_B already uses inverted frame clock
> for both CODEC and CPU.  In that case either both are wrong or it's just
> that the driver is limited in what it supports which is not a problem.

Well, I think the mcbsp module is quite - maybe too - flexible...
To have the DSP_B mode correctly (for the tvl320aic32 codec used in osk5912 
board) the FS polarity has to be handled by the mcbsp as it has been inverted. 
If we don't do this, there is no way to have the MSB at the correct place (it 
has to be available when the FS is high).

The DSP_A mode can use the FS polarity 'correctly' - as it is. Or we can also 
consider to require to invert the FS polarity, than add 1 bit delay for DSP_A 
mode.

So:
a) The proposal in the series
DSP_B mode (the MSB is transmitted when the FS is high, the length for the 
pulse is 1):
	case SND_SOC_DAIFMT_DSP_B:
		regs->srgr2	|= FPER(wlen * channels - 1);
		regs->srgr1	|= FWID(0);
		break;

	case SND_SOC_DAIFMT_DSP_B:
		/* 0-bit data delay */
		regs->rcr2      |= RDATDLY(0);
		regs->xcr2      |= XDATDLY(0);
		break;

	case SND_SOC_DAIFMT_NB_IF:
		regs->pcr0	|= CLKXP | CLKRP;
		break;

DSP_A mode (the MSB is transmitted when the FS went low, the length for the 
pulse is still 1, but the FS stays low for (wlen * channels - 1) cycles):
	case SND_SOC_DAIFMT_DSP_A:
		regs->srgr2	|= FPER(wlen * channels - 1);
		regs->srgr1	|= FWID(wlen * channels - 2);
		break;

	case SND_SOC_DAIFMT_DSP_A:
		/* 0-bit data delay */
		regs->rcr2      |= RDATDLY(0);
		regs->xcr2      |= XDATDLY(0);
		break;

	case SND_SOC_DAIFMT_IB_IF:
		break;

b) twist in DSP_A mode.
The DSP_B mode is the same as it is in a)

DSP_A mode (the MSB is transmitted when the FS went low, the length for the 
pulse is still 1, but the FS stays low for (wlen * channels - 1) cycles):
	case SND_SOC_DAIFMT_DSP_A:
		regs->srgr2	|= FPER(wlen * channels - 1);
		regs->srgr1	|= FWID(0);
		break;

	case SND_SOC_DAIFMT_DSP_A:
		/* 0-bit data delay */
		regs->rcr2      |= RDATDLY(1);
		regs->xcr2      |= XDATDLY(1);
		break;

	case SND_SOC_DAIFMT_NB_IF:
		regs->pcr0	|= CLKXP | CLKRP;
		break;

Both a) and b) would be OK for the DSP_A mode...

c)
Remove the SND_SOC_DAIFMT_* (DSP_A, DSP_B, I2S and NB_NF, NB_IF, IB_NF, IB_IF) 
configuration from the omap_mcbsp_dai_set_dai_fmt, move it to the 
omap_mcbsp_dai_hw_params function and have something like these there:

switch (mcbsp_data->fmt &
			(SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK)) {
case SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF:
	...
	break;
case SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_NF:
	...
	break;
case SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_IF:
	...
	break;
case SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF:
	...
	break;
case SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_IF:
	...
	break;
case SND_SOC_DAIFMT_DSP_A | SND_SOC_DAIFMT_NB_NF:
	...
	break;
...

}

and so on. I'm not sure if all of the combinations are valid, but it can be 
done like this also. It is another thing how actually the mcbsp would be 
configured for these modes - for DSP_B the FS polarity has to be inverted to 
get the MSB in a correct place, but it would be correct from the 
snd_soc_dai_set_fmt point of view, or something........


-- 
Péter

Re: [PATCH 1/2] ASoC: omap-mcbsp: Correct the?DSP_B mode

[Mark Brown]

On Tue, Apr 14, 2009 at 01:34:33PM +0300, Peter Ujfalusi wrote:

> To have the DSP_B mode correctly (for the tvl320aic32 codec used in osk5912 
> board) the FS polarity has to be handled by the mcbsp as it has been inverted. 
> If we don't do this, there is no way to have the MSB at the correct place (it 
> has to be available when the FS is high).

> The DSP_A mode can use the FS polarity 'correctly' - as it is. Or we can also 
> consider to require to invert the FS polarity, than add 1 bit delay for DSP_A 
> mode.

As Jarkko says the driver should be hiding all this from users - it
should just set the port up as best it can, refusing to do anything that
can't be supported by the hardware.  The default polarity the hardware
uses shouldn't be visible outside the driver.

> a) The proposal in the series
> DSP_B mode (the MSB is transmitted when the FS is high, the length for the 
> pulse is 1):

> DSP_A mode (the MSB is transmitted when the FS went low, the length for the 
> pulse is still 1, but the FS stays low for (wlen * channels - 1) cycles):

The difference between the two modes shouldn't be edge of FS used, it
should be a clock cycle, though with a 1 BCLK pulse on FS the effect
will probably line up.  The MSB data needs to be available for sampling
on the appropriate rising edge of BCLK.

Re: [PATCH 1/2] ASoC: omap-mcbsp: Correct the DSP_B mode

[Jarkko Nikula]

On Tue, 14 Apr 2009 12:34:33 +0200
"Ujfalusi Peter (Nokia-D/Tampere)" <peter.ujfalusi@nokia.com> wrote:

> Well, I think the mcbsp module is quite - maybe too - flexible...
> To have the DSP_B mode correctly (for the tvl320aic32 codec used in osk5912 board) the FS polarity has to be handled by the mcbsp as it has been inverted. If we don't do this, there is no way to have the MSB at the correct place (it has to be available when the FS is high).
> 
> 
> The DSP_A mode can use the FS polarity 'correctly' - as it is. Or we can also consider to require to invert the FS polarity, than add 1 bit delay for DSP_A mode.
> 
I meant something like this below integrating your FWID(0) and temp_fmt from my earlier commit da6320becf31c40b60d4b1dc6b339c9a766b671c so that SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_NF will produce correct format.

I tested this only with Beagle McBSP3 and oscilloscope but I like to verify it with real codecs as well :-)


Jarkko

diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
index d6882be..c40ea21 100644
--- a/sound/soc/omap/omap-mcbsp.c
+++ b/sound/soc/omap/omap-mcbsp.c
@@ -272,7 +272,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream,
 		break;
 	case SND_SOC_DAIFMT_DSP_B:
 		regs->srgr2	|= FPER(wlen * channels - 1);
-		regs->srgr1	|= FWID(wlen * channels - 2);
+		regs->srgr1	|= FWID(0);
 		break;
 	}
 
@@ -291,6 +291,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 {
 	struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data);
 	struct omap_mcbsp_reg_cfg *regs = &mcbsp_data->regs;
+	unsigned int temp_fmt = fmt;
 
 	if (mcbsp_data->configured)
 		return 0;
@@ -317,6 +318,8 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 		/* 0-bit data delay */
 		regs->rcr2      |= RDATDLY(0);
 		regs->xcr2      |= XDATDLY(0);
+		/* Invert bit clock and FS polarity configuration */
+		temp_fmt ^= SND_SOC_DAIFMT_IB_IF;
 		break;
 	default:
 		/* Unsupported data format */
@@ -340,7 +343,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai,
 	}
 
 	/* Set bit clock (CLKX/CLKR) and FS polarities */
-	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	switch (temp_fmt & SND_SOC_DAIFMT_INV_MASK) {
 	case SND_SOC_DAIFMT_NB_NF:
 		/*
 		 * Normal BCLK + FS.

Re: [PATCH 1/2] ASoC: omap-mcbsp: Correct the DSP_B mode

[Arun KS]

Hi,

I m not able to use my osk5912 to test this.
When i do an aplay with the latest kernel (linux-2.6.30-rc1) i am
getting an me i/o error.

Thanks,
Arun

On Tue, Apr 14, 2009 at 4:50 PM, Jarkko Nikula <jarkko.nikula@nokia.com> wrote:
> On Tue, 14 Apr 2009 12:34:33 +0200
> "Ujfalusi Peter (Nokia-D/Tampere)" <peter.ujfalusi@nokia.com> wrote:
>
>> Well, I think the mcbsp module is quite - maybe too - flexible...
>> To have the DSP_B mode correctly (for the tvl320aic32 codec used in osk5912 board) the FS polarity has to be handled by the mcbsp as it has been inverted. If we don't do this, there is no way to have the MSB at the correct place (it has to be available when the FS is high).
>>
>>
>> The DSP_A mode can use the FS polarity 'correctly' - as it is. Or we can also consider to require to invert the FS polarity, than add 1 bit delay for DSP_A mode.
>>
> I meant something like this below integrating your FWID(0) and temp_fmt from my earlier commit da6320becf31c40b60d4b1dc6b339c9a766b671c so that SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_NF will produce correct format.
>
> I tested this only with Beagle McBSP3 and oscilloscope but I like to verify it with real codecs as well :-)
>
>
> Jarkko
>
> diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
> index d6882be..c40ea21 100644
> --- a/sound/soc/omap/omap-mcbsp.c
> +++ b/sound/soc/omap/omap-mcbsp.c
> @@ -272,7 +272,7 @@ static int omap_mcbsp_dai_hw_params(struct snd_pcm_substream *substream,
>                break;
>        case SND_SOC_DAIFMT_DSP_B:
>                regs->srgr2     |= FPER(wlen * channels - 1);
> -               regs->srgr1     |= FWID(wlen * channels - 2);
> +               regs->srgr1     |= FWID(0);
>                break;
>        }
>
> @@ -291,6 +291,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai,
>  {
>        struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data);
>        struct omap_mcbsp_reg_cfg *regs = &mcbsp_data->regs;
> +       unsigned int temp_fmt = fmt;
>
>        if (mcbsp_data->configured)
>                return 0;
> @@ -317,6 +318,8 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai,
>                /* 0-bit data delay */
>                regs->rcr2      |= RDATDLY(0);
>                regs->xcr2      |= XDATDLY(0);
> +               /* Invert bit clock and FS polarity configuration */
> +               temp_fmt ^= SND_SOC_DAIFMT_IB_IF;
>                break;
>        default:
>                /* Unsupported data format */
> @@ -340,7 +343,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct snd_soc_dai *cpu_dai,
>        }
>
>        /* Set bit clock (CLKX/CLKR) and FS polarities */
> -       switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> +       switch (temp_fmt & SND_SOC_DAIFMT_INV_MASK) {
>        case SND_SOC_DAIFMT_NB_NF:
>                /*
>                 * Normal BCLK + FS.
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>

Re: [PATCH 1/2] ASoC: omap-mcbsp: Correct the DSP_B mode

[Peter Ujfalusi]

On Tuesday 14 April 2009 14:20:04 Nikula Jarkko (Nokia-D/Helsinki) wrote:
> On Tue, 14 Apr 2009 12:34:33 +0200
>
> I meant something like this below integrating your FWID(0) and temp_fmt
> from my earlier commit da6320becf31c40b60d4b1dc6b339c9a766b671c so that
> SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_NF will produce correct format.
>
> I tested this only with Beagle McBSP3 and oscilloscope but I like to verify
> it with real codecs as well :-)
>
>
> Jarkko
>
> diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
> index d6882be..c40ea21 100644
> --- a/sound/soc/omap/omap-mcbsp.c
> +++ b/sound/soc/omap/omap-mcbsp.c
> @@ -272,7 +272,7 @@ static int omap_mcbsp_dai_hw_params(struct
> snd_pcm_substream *substream, break;
>  	case SND_SOC_DAIFMT_DSP_B:
>  		regs->srgr2	|= FPER(wlen * channels - 1);
> -		regs->srgr1	|= FWID(wlen * channels - 2);
> +		regs->srgr1	|= FWID(0);
>  		break;
>  	}
>
> @@ -291,6 +291,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct
> snd_soc_dai *cpu_dai, {
>  	struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data);
>  	struct omap_mcbsp_reg_cfg *regs = &mcbsp_data->regs;
> +	unsigned int temp_fmt = fmt;
>
>  	if (mcbsp_data->configured)
>  		return 0;
> @@ -317,6 +318,8 @@ static int omap_mcbsp_dai_set_dai_fmt(struct
> snd_soc_dai *cpu_dai, /* 0-bit data delay */
>  		regs->rcr2      |= RDATDLY(0);
>  		regs->xcr2      |= XDATDLY(0);
> +		/* Invert bit clock and FS polarity configuration */
> +		temp_fmt ^= SND_SOC_DAIFMT_IB_IF;
>  		break;
>  	default:
>  		/* Unsupported data format */
> @@ -340,7 +343,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct
> snd_soc_dai *cpu_dai, }
>
>  	/* Set bit clock (CLKX/CLKR) and FS polarities */
> -	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> +	switch (temp_fmt & SND_SOC_DAIFMT_INV_MASK) {
>  	case SND_SOC_DAIFMT_NB_NF:
>  		/*
>  		 * Normal BCLK + FS.

In case of the osk5912 board with tlv320aic32 codec, than you need to 
configure it like this?

snd_soc_dai_set_fmt(cpu_dai,
	SND_SOC_DAIFMT_DSP_B |
	SND_SOC_DAIFMT_IB_NF | /* invert bclk + nor frm */
	SND_SOC_DAIFMT_CBM_CFM);


Btw, what does:
	inverted bclk
	normal bclk
	inverted frm
	normal frm
actually means?

Is it so that it is:
inverted bclk:
	sample is driven on the rising edge, sampled on the falling edge
normal bclk:
	sample is driven on the falling edge, sampled on the rising edge
inverted frm:
	FS is active high
normal frm:
	FS is active low

-- 
Péter

Re: [PATCH 1/2] ASoC: omap-mcbsp: Correct the DSP_B mode

[Mark Brown]

On Tue, Apr 14, 2009 at 02:56:49PM +0300, Peter Ujfalusi wrote:

> Is it so that it is:
> inverted bclk:
> 	sample is driven on the rising edge, sampled on the falling edge
> normal bclk:
> 	sample is driven on the falling edge, sampled on the rising edge

Yes.

> inverted frm:
> 	FS is active high
> normal frm:
> 	FS is active low

Ish.  For DSP mode normal frame means that things are synced of the
rising edge of the frame clock so inverted frame should mean the falling
edge.  Frame clock is allowed to be longer than one BCLK cycle, though
it is not normally and some things may need the low limit.

Re: [PATCH 1/2] ASoC: omap-mcbsp: Correct the DSP_B mode

[Peter Ujfalusi]

On Tuesday 14 April 2009 14:20:04 Nikula Jarkko (Nokia-D/Helsinki) wrote:
> On Tue, 14 Apr 2009 12:34:33 +0200
>
> "Ujfalusi Peter (Nokia-D/Tampere)" <peter.ujfalusi@nokia.com> wrote:
> > Well, I think the mcbsp module is quite - maybe too - flexible...
> > To have the DSP_B mode correctly (for the tvl320aic32 codec used in
> > osk5912 board) the FS polarity has to be handled by the mcbsp as it has
> > been inverted. If we don't do this, there is no way to have the MSB at
> > the correct place (it has to be available when the FS is high).
> >
> >
> > The DSP_A mode can use the FS polarity 'correctly' - as it is. Or we can
> > also consider to require to invert the FS polarity, than add 1 bit delay
> > for DSP_A mode.
>
> I meant something like this below integrating your FWID(0) and temp_fmt
> from my earlier commit da6320becf31c40b60d4b1dc6b339c9a766b671c so that
> SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_NB_NF will produce correct format.
>
> I tested this only with Beagle McBSP3 and oscilloscope but I like to verify
> it with real codecs as well :-)
>
>
> Jarkko
>
> diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
> index d6882be..c40ea21 100644
> --- a/sound/soc/omap/omap-mcbsp.c
> +++ b/sound/soc/omap/omap-mcbsp.c
> @@ -272,7 +272,7 @@ static int omap_mcbsp_dai_hw_params(struct
> snd_pcm_substream *substream, break;
>  	case SND_SOC_DAIFMT_DSP_B:
>  		regs->srgr2	|= FPER(wlen * channels - 1);
> -		regs->srgr1	|= FWID(wlen * channels - 2);
> +		regs->srgr1	|= FWID(0);
>  		break;
>  	}
>
> @@ -291,6 +291,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct
> snd_soc_dai *cpu_dai, {
>  	struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data);
>  	struct omap_mcbsp_reg_cfg *regs = &mcbsp_data->regs;
> +	unsigned int temp_fmt = fmt;
>
>  	if (mcbsp_data->configured)
>  		return 0;
> @@ -317,6 +318,8 @@ static int omap_mcbsp_dai_set_dai_fmt(struct
> snd_soc_dai *cpu_dai, /* 0-bit data delay */
>  		regs->rcr2      |= RDATDLY(0);
>  		regs->xcr2      |= XDATDLY(0);
> +		/* Invert bit clock and FS polarity configuration */
> +		temp_fmt ^= SND_SOC_DAIFMT_IB_IF;

Should it be like this instead:
 +		temp_fmt ^= SND_SOC_DAIFMT_NB_IF;

We only need to invert the FS polarity and the bclk should stay as it is..
Also should than this modified fmt stored to mcbsp_data->fmt later?


>  		break;
>  	default:
>  		/* Unsupported data format */
> @@ -340,7 +343,7 @@ static int omap_mcbsp_dai_set_dai_fmt(struct
> snd_soc_dai *cpu_dai, }
>
>  	/* Set bit clock (CLKX/CLKR) and FS polarities */
> -	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> +	switch (temp_fmt & SND_SOC_DAIFMT_INV_MASK) {
>  	case SND_SOC_DAIFMT_NB_NF:
>  		/*
>  		 * Normal BCLK + FS.

-- 
Péter

Re: [PATCH 1/2] ASoC: omap-mcbsp: Correct the DSP_B mode

[Jarkko Nikula]

On Tue, 14 Apr 2009 14:32:30 +0200
"Ujfalusi Peter (Nokia-D/Tampere)" <peter.ujfalusi@nokia.com> wrote:

> Should it be like this instead:
> + temp_fmt ^= SND_SOC_DAIFMT_NB_IF;
> 
Absolutely yes, good catch :-)

> 
> We only need to invert the FS polarity and the bclk should stay as it is..
> Also should than this modified fmt stored to mcbsp_data->fmt later?
> 
No since it's used only inside omap_mcbsp_dai_set_dai_fmt to tune polarity for the mode default. That's why there is this temp_fmt variable.


Jarkko

Re: [PATCH 1/2] ASoC: omap-mcbsp: Correct the DSP_B mode

[Jarkko Nikula]

On Tue, 14 Apr 2009 13:32:45 +0200
ext Arun KS <getarunks@gmail.com> wrote:

> Hi,
> 
> I m not able to use my osk5912 to test this.
> When i do an aplay with the latest kernel (linux-2.6.30-rc1) i am
> getting an me i/o error.
> 
Are you using mainline or linux-omap? It seems that linux-omap is somewhat broken at the moment. I got Beagle working by going back into commit 823de921c923cc753a6746183e9c513a3452dec7 but not yet figured out one for N810...


Jarkko

Re: [PATCH 1/2] ASoC: omap-mcbsp: Correct the DSP_B mode

[Peter Ujfalusi]

On Tuesday 14 April 2009 15:49:00 Nikula Jarkko (Nokia-D/Helsinki) wrote:
> On Tue, 14 Apr 2009 14:32:30 +0200
>
> "Ujfalusi Peter (Nokia-D/Tampere)" <peter.ujfalusi@nokia.com> wrote:
> > Should it be like this instead:
> > + temp_fmt ^= SND_SOC_DAIFMT_NB_IF;
>
> Absolutely yes, good catch :-)
>
> > We only need to invert the FS polarity and the bclk should stay as it
> > is.. Also should than this modified fmt stored to mcbsp_data->fmt later?
>
> No since it's used only inside omap_mcbsp_dai_set_dai_fmt to tune polarity
> for the mode default. That's why there is this temp_fmt variable.

Fair enough.

Could you resend the patch?
I think this is a good starting point for getting this DSP_A/_B mode right, 
finally...
I'll can than send the DSP_A patch on top of your DSP_B... Than I can resend 
the TWL4030 four channel patches on top of these ;)

>
>
> Jarkko

-- 
Péter