Medisetty, Naresh wrote:
> Troy,
>
>
>> -----Original Message-----
>> From: [email protected]
>> [mailto:[email protected]] On Behalf
>> Of Troy Kisky
>> Sent: Saturday, December 13, 2008 8:39 AM
>> To: [email protected]
>> Subject: [PATCH] (v2)RFC: ARM: DaVinci: ASoC: dm644x_evm use dsp_a
>>
>> Currently, the aic33 codec is set to use I2S mode
>> and the cpu is set to use dsp_b mode. Although,
>> this seems to work if the codec is master. It is
>> very confusing. Change both codec and cpu to
>> use dsp_a mode.
>>
>> Also fixed davinci-i2s to configure
>> SND_SOC_DAIFMT_NB_NF as data valid on rising edge of
>> CLKX, CLKR
>>
>> Correspondingly, SND_SOC_DAIFMT_IB_NF now means data
>> valid on falling edge of CLKX, CLKR
>>
>> Signed-off-by: Troy Kisky <[email protected]>
>> --- a/sound/soc/davinci/davinci-evm.c
>> +++ b/sound/soc/davinci/davinci-evm.c
>> @@ -31,6 +31,9 @@
>>
>> #define EVM_CODEC_CLOCK 22579200
>>
>> +#define AUDIO_FORMAT (SND_SOC_DAIFMT_DSP_A | \
>> + SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_IB_NF)
>> +
>
> Support as many formats as possible, since some low end codec support only I2S
Since davinci only supports dsp mode, it should request dsp mode.
If other codecs only support I2S, then they can play games
and request dsp_b mode from the cpu, and I2S mode from the codec.
That however should NOT be the default.
>
>> static int evm_hw_params(struct snd_pcm_substream *substream,
>> struct snd_pcm_hw_params *params)
>> {
>> @@ -40,14 +43,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);
>> + 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_CBM_CFM |
>> - SND_SOC_DAIFMT_IB_NF);
>> + ret = snd_soc_dai_set_fmt(cpu_dai, AUDIO_FORMAT);
>> if (ret < 0)
>> return ret;
>>
>
>> switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
>> case SND_SOC_DAIFMT_IB_NF:
>> - 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);
>> + /* 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;
>> - case SND_SOC_DAIFMT_NB_IF:
>> - 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);
>> + case SND_SOC_DAIFMT_NB_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
>> + */
>> + pcr |= (DAVINCI_MCBSP_PCR_CLKXP | DAVINCI_MCBSP_PCR_CLKRP);
>> break;
>> case SND_SOC_DAIFMT_IB_IF:
>> - 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);
>> + /* 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
>> + */
>> + pcr |= (DAVINCI_MCBSP_PCR_FSXP | DAVINCI_MCBSP_PCR_FSRP);
>> break;
>> - case SND_SOC_DAIFMT_NB_NF:
>> + case SND_SOC_DAIFMT_NB_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
>> + */
>> + pcr |= (DAVINCI_MCBSP_PCR_CLKXP | DAVINCI_MCBSP_PCR_CLKRP |
>> + DAVINCI_MCBSP_PCR_FSXP | DAVINCI_MCBSP_PCR_FSRP);
>> break;
>> default:
>> return -EINVAL;
>> }
>
> I feel Bit and frame masks are proper in the existing code with respect to ASP
>
Frame mask yes, BCLK no way. So, you think it makes sense to tell the codec that
data is valid on the rising edge (for the original I2S mode) and to tell the CPU
that you want data to be valid on the falling edge?
Don't you think it makes more sense to tell both the same thing?
This is just so obvious to me that I'm in a state of shock that you
would disagree.
Troy
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
