From: Mark Brown on
On Wed, Jun 30, 2010 at 03:47:56PM +0200, Raffaele Recalcati wrote:

> +static int davinci_i2s_dai_set_clkdiv(struct snd_soc_dai *cpu_dai,
> + int div_id, int div)
> +{
> + struct davinci_mcbsp_dev *dev = cpu_dai->private_data;
> + int srgr;
> +
> + dev->clk_div = div;
> + return 0;
> +}

As previously mentioned this should check the div_id argument.

> + switch (master) {
> + case SND_SOC_DAIFMT_CBS_CFS:

....

> + case SND_SOC_DAIFMT_CBM_CFS:

....

> + default:
> + /* Clock and frame sync given from external sources */

The two remaining options (_CBM_CFM and _CBS_CFM) are different, your
description matches _CBM_CFM.

> + if (master == SND_SOC_DAIFMT_CBS_CFS ||
> + master == SND_SOC_DAIFMT_CBS_CFM) {

Switch statement again.

> + if (master == SND_SOC_DAIFMT_CBS_CFS ||
> + master == SND_SOC_DAIFMT_CBS_CFM) {
> + rcr |= DAVINCI_MCBSP_RCR_RFRLEN1(0);
> + xcr |= DAVINCI_MCBSP_XCR_XFRLEN1(0);
> + } else {

....and another. Please fix all these.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Liam Girdwood on
On Tue, 2010-07-06 at 10:39 +0200, Raffaele Recalcati wrote:
> From: Raffaele Recalcati <raffaele.recalcati(a)bticino.it>
>
> Added two clocking options for dm365 McBSP peripheral when used
> with I2S timings, that are SND_SOC_DAIFMT_CBS_CFS (the cpu generates
> clock and frame sync) and SND_SOC_DAIFMT_CBS_CFM (the cpu gets clock
> from external pin and generates frame sync).
> A slave clock management can be important when the external codec needs
> the system clock and the bit clock synchronized (tested with uda1345).
> This patch has been developed against the:
> http://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-davinci.git
> git tree and has been tested on bmx board (similar to dm365 evm, but using
> uda1345 as external audio codec).
>
> Signed-off-by: Raffaele Recalcati <raffaele.recalcati(a)bticino.it>
> Signed-off-by: Davide Bonfanti <davide.bonfanti(a)bticino.it>

All

Acked-by: Liam Girdwood <lrg(a)slimlogic.co.uk>

But lets get some feedback from the DaVinci folks on this too.

> ---
> sound/soc/davinci/davinci-i2s.c | 110 +++++++++++++++++++++++++++++++++++---
> sound/soc/davinci/davinci-i2s.h | 5 ++
> 2 files changed, 106 insertions(+), 9 deletions(-)
>
> diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
> index adadcd3..c8f038c 100644
> --- a/sound/soc/davinci/davinci-i2s.c
> +++ b/sound/soc/davinci/davinci-i2s.c
> @@ -26,6 +26,7 @@
> #include <mach/asp.h>
>
> #include "davinci-pcm.h"
> +#include "davinci-i2s.h"
>
>
> /*
> @@ -68,16 +69,21 @@
> #define DAVINCI_MCBSP_RCR_RDATDLY(v) ((v) << 16)
> #define DAVINCI_MCBSP_RCR_RFIG (1 << 18)
> #define DAVINCI_MCBSP_RCR_RWDLEN2(v) ((v) << 21)
> +#define DAVINCI_MCBSP_RCR_RFRLEN2(v) ((v) << 24)
> +#define DAVINCI_MCBSP_RCR_RPHASE BIT(31)
>
> #define DAVINCI_MCBSP_XCR_XWDLEN1(v) ((v) << 5)
> #define DAVINCI_MCBSP_XCR_XFRLEN1(v) ((v) << 8)
> #define DAVINCI_MCBSP_XCR_XDATDLY(v) ((v) << 16)
> #define DAVINCI_MCBSP_XCR_XFIG (1 << 18)
> #define DAVINCI_MCBSP_XCR_XWDLEN2(v) ((v) << 21)
> +#define DAVINCI_MCBSP_XCR_XFRLEN2(v) ((v) << 24)
> +#define DAVINCI_MCBSP_XCR_XPHASE BIT(31)
>
> #define DAVINCI_MCBSP_SRGR_FWID(v) ((v) << 8)
> #define DAVINCI_MCBSP_SRGR_FPER(v) ((v) << 16)
> #define DAVINCI_MCBSP_SRGR_FSGM (1 << 28)
> +#define DAVINCI_MCBSP_SRGR_CLKSM BIT(29)
>
> #define DAVINCI_MCBSP_PCR_CLKRP (1 << 0)
> #define DAVINCI_MCBSP_PCR_CLKXP (1 << 1)
> @@ -144,6 +150,9 @@ struct davinci_mcbsp_dev {
> * won't end up being swapped because of the underrun.
> */
> unsigned enable_channel_combine:1;
> +
> + unsigned int fmt;
> + int clk_div;
> };
>
> static inline void davinci_mcbsp_write_reg(struct davinci_mcbsp_dev *dev,
> @@ -254,10 +263,12 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
> struct davinci_mcbsp_dev *dev = cpu_dai->private_data;
> unsigned int pcr;
> unsigned int srgr;
> + /* Attention srgr is updated by hw_params! */
> srgr = DAVINCI_MCBSP_SRGR_FSGM |
> DAVINCI_MCBSP_SRGR_FPER(DEFAULT_BITPERSAMPLE * 2 - 1) |
> DAVINCI_MCBSP_SRGR_FWID(DEFAULT_BITPERSAMPLE - 1);
>
> + dev->fmt = fmt;
> /* set master/slave audio interface */
> switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> case SND_SOC_DAIFMT_CBS_CFS:
> @@ -372,6 +383,18 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai *cpu_dai,
> return 0;
> }
>
> +static int davinci_i2s_dai_set_clkdiv(struct snd_soc_dai *cpu_dai,
> + int div_id, int div)
> +{
> + struct davinci_mcbsp_dev *dev = cpu_dai->private_data;
> +
> + if (div_id != DAVINCI_MCBSP_CLKGDV)
> + return -ENODEV;
> +
> + dev->clk_div = div;
> + return 0;
> +}
> +
> static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
> struct snd_pcm_hw_params *params,
> struct snd_soc_dai *dai)
> @@ -380,8 +403,8 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
> struct davinci_pcm_dma_params *dma_params =
> &dev->dma_params[substream->stream];
> struct snd_interval *i = NULL;
> - int mcbsp_word_length;
> - unsigned int rcr, xcr, srgr;
> + int mcbsp_word_length, master;
> + unsigned int rcr, xcr, srgr, clk_div, freq, framesize;
> u32 spcr;
> snd_pcm_format_t fmt;
> unsigned element_cnt = 1;
> @@ -396,12 +419,47 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
> davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SPCR_REG, spcr);
> }
>
> - i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS);
> - srgr = DAVINCI_MCBSP_SRGR_FSGM;
> - srgr |= DAVINCI_MCBSP_SRGR_FWID(snd_interval_value(i) - 1);
> + master = dev->fmt & SND_SOC_DAIFMT_MASTER_MASK;
> + fmt = params_format(params);
> + mcbsp_word_length = asp_word_length[fmt];
>
> - i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_FRAME_BITS);
> - srgr |= DAVINCI_MCBSP_SRGR_FPER(snd_interval_value(i) - 1);
> + switch (master) {
> + case SND_SOC_DAIFMT_CBS_CFS:
> + freq = clk_get_rate(dev->clk);
> + srgr = DAVINCI_MCBSP_SRGR_FSGM |
> + DAVINCI_MCBSP_SRGR_CLKSM;
> + srgr |= DAVINCI_MCBSP_SRGR_FWID(mcbsp_word_length *
> + 8 - 1);
> + /* symmetric waveforms */
> + clk_div = freq / (mcbsp_word_length * 16) /
> + params->rate_num * params->rate_den;
> + srgr |= DAVINCI_MCBSP_SRGR_FPER(mcbsp_word_length *
> + 16 - 1);
> + clk_div &= 0xFF;
> + srgr |= clk_div;
> + break;
> + case SND_SOC_DAIFMT_CBM_CFS:
> + srgr = DAVINCI_MCBSP_SRGR_FSGM;
> + clk_div = dev->clk_div - 1;
> + srgr |= DAVINCI_MCBSP_SRGR_FWID(mcbsp_word_length * 8 - 1);
> + srgr |= DAVINCI_MCBSP_SRGR_FPER(mcbsp_word_length * 16 - 1);
> + clk_div &= 0xFF;
> + srgr |= clk_div;
> + break;
> + case SND_SOC_DAIFMT_CBM_CFM:
> + /* Clock and frame sync given from external sources */
> + i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS);
> + srgr = DAVINCI_MCBSP_SRGR_FSGM;
> + srgr |= DAVINCI_MCBSP_SRGR_FWID(snd_interval_value(i) - 1);
> + pr_debug("%s - %d FWID set: re-read srgr = %X\n",
> + __func__, __LINE__, snd_interval_value(i) - 1);
> +
> + i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_FRAME_BITS);
> + srgr |= DAVINCI_MCBSP_SRGR_FPER(snd_interval_value(i) - 1);
> + break;
> + default:
> + return -EINVAL;
> + }
> davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr);
>
> rcr = DAVINCI_MCBSP_RCR_RFIG;
> @@ -426,12 +484,41 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
> element_cnt = 1;
> fmt = double_fmt[fmt];
> }
> + switch (master) {
> + case SND_SOC_DAIFMT_CBS_CFS:
> + case SND_SOC_DAIFMT_CBS_CFM:
> + rcr |= DAVINCI_MCBSP_RCR_RFRLEN2(0);
> + xcr |= DAVINCI_MCBSP_XCR_XFRLEN2(0);
> + rcr |= DAVINCI_MCBSP_RCR_RPHASE;
> + xcr |= DAVINCI_MCBSP_XCR_XPHASE;
> + break;
> + case SND_SOC_DAIFMT_CBM_CFM:
> + case SND_SOC_DAIFMT_CBM_CFS:
> + rcr |= DAVINCI_MCBSP_RCR_RFRLEN2(element_cnt - 1);
> + xcr |= DAVINCI_MCBSP_XCR_XFRLEN2(element_cnt - 1);
> + break;
> + default:
> + return -EINVAL;
> + }
> }
> dma_params->acnt = dma_params->data_type = data_type[fmt];
> dma_params->fifo_level = 0;
> mcbsp_word_length = asp_word_length[fmt];
> - rcr |= DAVINCI_MCBSP_RCR_RFRLEN1(element_cnt - 1);
> - xcr |= DAVINCI_MCBSP_XCR_XFRLEN1(element_cnt - 1);
> +
> + switch (master) {
> + case SND_SOC_DAIFMT_CBS_CFS:
> + case SND_SOC_DAIFMT_CBS_CFM:
> + rcr |= DAVINCI_MCBSP_RCR_RFRLEN1(0);
> + xcr |= DAVINCI_MCBSP_XCR_XFRLEN1(0);
> + break;
> + case SND_SOC_DAIFMT_CBM_CFM:
> + case SND_SOC_DAIFMT_CBM_CFS:
> + rcr |= DAVINCI_MCBSP_RCR_RFRLEN1(element_cnt - 1);
> + xcr |= DAVINCI_MCBSP_XCR_XFRLEN1(element_cnt - 1);
> + break;
> + default:
> + return -EINVAL;
> + }
>
> rcr |= DAVINCI_MCBSP_RCR_RWDLEN1(mcbsp_word_length) |
> DAVINCI_MCBSP_RCR_RWDLEN2(mcbsp_word_length);
> @@ -442,6 +529,10 @@ static int davinci_i2s_hw_params(struct snd_pcm_substream *substream,
> davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_XCR_REG, xcr);
> else
> davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_RCR_REG, rcr);
> +
> + pr_debug("%s - %d srgr=%X\n", __func__, __LINE__, srgr);
> + pr_debug("%s - %d xcr=%X\n", __func__, __LINE__, xcr);
> + pr_debug("%s - %d rcr=%X\n", __func__, __LINE__, rcr);
> return 0;
> }
>
> @@ -500,6 +591,7 @@ static struct snd_soc_dai_ops davinci_i2s_dai_ops = {
> .trigger = davinci_i2s_trigger,
> .hw_params = davinci_i2s_hw_params,
> .set_fmt = davinci_i2s_set_dai_fmt,
> + .set_clkdiv = davinci_i2s_dai_set_clkdiv,
>
> };
>
> diff --git a/sound/soc/davinci/davinci-i2s.h b/sound/soc/davinci/davinci-i2s.h
> index 241648c..0b1e77b 100644
> --- a/sound/soc/davinci/davinci-i2s.h
> +++ b/sound/soc/davinci/davinci-i2s.h
> @@ -12,6 +12,11 @@
> #ifndef _DAVINCI_I2S_H
> #define _DAVINCI_I2S_H
>
> +/* McBSP dividers */
> +enum davinci_mcbsp_div {
> + DAVINCI_MCBSP_CLKGDV, /* Sample rate generator divider */
> +};
> +
> extern struct snd_soc_dai davinci_i2s_dai;
>
> #endif

--
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Mark Brown on
On Tue, Jul 06, 2010 at 05:00:24PM +0530, Sudhakar Rajashekhara wrote:
> > Acked-by: Liam Girdwood <lrg(a)slimlogic.co.uk>

> Acked-by: Sudhakar Rajashekhara <sudhakar.raj(a)ti.com>

Applied all, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/