> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]] On Behalf
> Of Troy Kisky
> Sent: Tuesday, December 16, 2008 12:46 AM
> To: Medisetty, Naresh
> Cc: [email protected]
> Subject: Re: [PATCH] (v2)RFC: ARM: DaVinci: ASoC: dm644x_evm use dsp_a
>
> 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.
>
Davinci can support all the audio modes including I2S.
>
> >
> >> 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?
>
>
In Davinci ASP normal BCLK means data is valid on falling edge and normal frame
sync means active high.
For AIC33 there is no need to tell frame sync and bit polarities since it is
directly configurable and polarities are dependent on the modes whether I2S or
DSP.
So adding the following comment on the existing code will rule out the confusion
/*
* Normal BCLK + FS.
* FS active high. TX data driven on rising edge of bit clock
* and RX data sampled on falling edge of bit clock.
*/
> 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.
>
>
I think by telling the codec only the mode whether I2S or DSP and configuring
the polarities of ASP according to the mode will make more sense.
> Troy
>
>
> _______________________________________________
> Davinci-linux-open-source mailing list
> [email protected]
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
