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