Raffaele Recalcati <[email protected]> writes:
> From: Raffaele Recalcati <[email protected]>
>
> - Frame Sync master and Clock master (internally generated)
> - Frame Sync master and Clock slave
>
> Signed-off-by: Raffaele Recalcati <[email protected]>
> Signed-off-by: Davide Bonfanti <[email protected]>
> ---
First, some general comments on your series.
1) Grouping patches together in series is usually done to suggest
interdependencies, or that all the patches go together towards a
single problem/feature etc. Your current series combines multiple
patches, most of which are independent and most of which need to go
upstream through different subsytems.
For example, the audio/asoc patches go upstream through the ASoC
maintainers, and the SPI, touchscreen, mfd, MTD drivers all have
different subsystems and maintainers.
The only thing that gets merged directly through me and the davinci
git tree are changes to arch/arm/mach-davinci/*.
So, I suggest you break this up into patches that are per-subsytem.
In other words, one patch/series for arch/arm/mach-davinci/* another
for audio, another for video, etc.
To find out the right mailing lists for each of these subsystems,
please consult the MAINTAINERS file. A really useful tool is the
scripts/get_maintainer.pl script. Running that on a patch will
suggest the right audience for that patch using the MAINTAINERS file.
Since I gathered you're now using git-send-email, you can use this script
in combination with git-send-email's --cc-cmd option. e.g.
git-send-email --to <davinci list> --cc-cmd=scripts/get_maintainer.pl [...]
will automatically add the right folks to the Cc: line of the emails
sent by git-send-email.
2) For each series, please be sure to state what tree the patches
were generated against, and on what platforms they were tested.
3) Please add descriptive changelogs to each patch. The changelog
should describe the problem being addressed, the solution and
why.
Some other comments below on this patch...
> sound/soc/davinci/davinci-i2s.c | 120 +++++++++++++++++++++++++++++++++++---
> 1 files changed, 110 insertions(+), 10 deletions(-)
>
> diff --git a/sound/soc/davinci/davinci-i2s.c b/sound/soc/davinci/davinci-i2s.c
> index adadcd3..620f977 100644
> --- a/sound/soc/davinci/davinci-i2s.c
> +++ b/sound/soc/davinci/davinci-i2s.c
> @@ -68,16 +68,23 @@
> #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 (1 << 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 (1 << 31)
>
> +
> +#define CLKGDV(v) (v) /* Bits 0:7 */
> #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 (1 << 29)
>
> #define DAVINCI_MCBSP_PCR_CLKRP (1 << 0)
> #define DAVINCI_MCBSP_PCR_CLKXP (1 << 1)
> @@ -89,6 +96,13 @@
> #define DAVINCI_MCBSP_PCR_FSRM (1 << 10)
> #define DAVINCI_MCBSP_PCR_FSXM (1 << 11)
>
> +/*
> + * This define works when both clock and FS are output for the cpu
> + * and makes clock very fast (FS is not simmetrical, but sampling
> + * frequency is better approximated
> + */
> +#define I2S_FAST_CLOCK
> +
> enum {
> DAVINCI_MCBSP_WORD_8 = 0,
> DAVINCI_MCBSP_WORD_12,
> @@ -146,6 +160,13 @@ struct davinci_mcbsp_dev {
> unsigned enable_channel_combine:1;
> };
>
> +struct davinci_mcbsp_data {
> + unsigned int fmt;
this indent is a tab (as it should be)
> + int clk_div;
and this is spaces. Please use tabs throughout.
> +};
> +
> +static struct davinci_mcbsp_data mcbsp_data;
> +
> static inline void davinci_mcbsp_write_reg(struct davinci_mcbsp_dev *dev,
> int reg, u32 val)
> {
> @@ -257,9 +278,12 @@ static int davinci_i2s_set_dai_fmt(struct snd_soc_dai
> *cpu_dai,
> srgr = DAVINCI_MCBSP_SRGR_FSGM |
> DAVINCI_MCBSP_SRGR_FPER(DEFAULT_BITPERSAMPLE * 2 - 1) |
> DAVINCI_MCBSP_SRGR_FWID(DEFAULT_BITPERSAMPLE - 1);
> + /* Attention srgr is updated by hw_params! */
>
> + mcbsp_data.fmt = fmt;
> /* set master/slave audio interface */
> switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> + case SND_SOC_DAIFMT_CBS_CFM:
> case SND_SOC_DAIFMT_CBS_CFS:
> /* cpu is master */
> pcr = DAVINCI_MCBSP_PCR_FSXM |
> @@ -372,6 +396,17 @@ 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;
> + int srgr;
> +
> + mcbsp_data.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,11 +415,12 @@ 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;
> + struct clk *clk;
>
> /* general line settings */
> spcr = davinci_mcbsp_read_reg(dev, DAVINCI_MCBSP_SPCR_REG);
> @@ -396,12 +432,53 @@ 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 = mcbsp_data.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);
> + if (master == SND_SOC_DAIFMT_CBS_CFS) {
> + clk = clk_get(NULL, "pll1_sysclk6");
> + if (clk)
> + freq = clk_get_rate(clk);
> + freq = 122000000; /* FIXME ask to Texas */
> +#ifdef I2S_FAST_CLOCK
> + clk_div = 256;
> + do {
> + framesize = (freq / (--clk_div)) / params->rate_num * \
> + params->rate_den;
> + } while (((framesize < 33) || (framesize > 4095)) && (clk_div));
> + clk_div--;
> + srgr = DAVINCI_MCBSP_SRGR_FSGM | DAVINCI_MCBSP_SRGR_CLKSM;
> + srgr |= DAVINCI_MCBSP_SRGR_FWID(mcbsp_word_length*8-1);
> + srgr |= DAVINCI_MCBSP_SRGR_FPER(framesize - 1);
> +#else
> + /* simmetric waveforms */
> + srgr = DAVINCI_MCBSP_SRGR_FSGM | DAVINCI_MCBSP_SRGR_CLKSM;
> + srgr |= DAVINCI_MCBSP_SRGR_FWID(mcbsp_word_length * 8 - 1);
> + srgr |= DAVINCI_MCBSP_SRGR_FPER(mcbsp_word_length * 16 - 1);
> + clk_div = freq / (mcbsp_word_length * 16) / params->rate_num * \
> + params->rate_den;
> +#endif
These optional features are better done using run-time decisions based
on optional flags passed in from board files instead of compile-time
decisions using #ifdef.
> + clk_div &= 0xFF;
> + srgr |= clk_div;
> + } else if (master == SND_SOC_DAIFMT_CBS_CFM) {
> + /* Clock given on CLKS */
> + srgr = DAVINCI_MCBSP_SRGR_FSGM;
> + clk_div = mcbsp_data.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;
> + } else {
> + 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);
> + }
> davinci_mcbsp_write_reg(dev, DAVINCI_MCBSP_SRGR_REG, srgr);
>
> rcr = DAVINCI_MCBSP_RCR_RFIG;
> @@ -426,12 +503,31 @@ static int davinci_i2s_hw_params(struct
> snd_pcm_substream *substream,
> element_cnt = 1;
> fmt = double_fmt[fmt];
> }
> + if (master == SND_SOC_DAIFMT_CBS_CFS ||
> + master == 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;
> + } else {
> + /* FIXME ask to Texas */
> + rcr |= DAVINCI_MCBSP_RCR_RFRLEN2(element_cnt - 1);
> + xcr |= DAVINCI_MCBSP_XCR_XFRLEN2(element_cnt - 1);
> + }
> }
> 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);
> +
> + 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 {
> + /* FIXME ask to Texas */
> + rcr |= DAVINCI_MCBSP_RCR_RFRLEN1(element_cnt - 1);
> + xcr |= DAVINCI_MCBSP_XCR_XFRLEN1(element_cnt - 1);
> + }
>
> rcr |= DAVINCI_MCBSP_RCR_RWDLEN1(mcbsp_word_length) |
> DAVINCI_MCBSP_RCR_RWDLEN2(mcbsp_word_length);
> @@ -442,6 +538,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,7 +600,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,
please keep alignment
Kevin
> };
>
> struct snd_soc_dai davinci_i2s_dai = {
> --
> 1.7.0.4
>
> _______________________________________________
> 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
