Hi Kevin, first of all thanks for your reply. I'm very happy to see a feedback.
2010/6/22 Kevin Hilman <khil...@deeprootsystems.com> > Raffaele Recalcati <lamiapost...@gmail.com> writes: > > > From: Raffaele Recalcati <raffaele.recalc...@bticino.it> > > > > - Frame Sync master and Clock master (internally generated) > > - Frame Sync master and Clock slave > > > > Signed-off-by: Raffaele Recalcati <raffaele.recalc...@bticino.it> > > Signed-off-by: Davide Bonfanti <davide.bonfa...@bticino.it> > > --- > > 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. > It will be done. > > 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. > Ok, but what about sound/soc/davinci and similar directories? Anyway ... the patches are sent also to davinci mailing list and you can see them. > > 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. > I will write for example: The patch has been created against http://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-davinci.git and checked with bmx dm365 board (similar to evm dm365). > 3) Please add descriptive changelogs to each patch. The changelog > should describe the problem being addressed, the solution and > why. > It will be done. > > 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. > I had a checkpatch false positive. I have fixed it sending this patch to its mantainers. Sorry, but this mail client (gmail with webmail) converts tabs in spaces so the following patch is not directly usable. diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index a4d7434..315a827 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1433,6 +1433,13 @@ sub process { WARN("please, no space before tabs\n" . $herevet); } +# check for spaces at the beginning of a line. + if ($rawline =~ /^\+ / && $rawline !~ /\+ +\*/) { + my $herevet = "$here\n" . cat_vet($rawline) . "\n"; + WARN("please, no space for starting a line, \ + excluding comments\n" . $herevet); + } + # check we are in a valid C source file if not then ignore this hunk next if ($realfile !~ /\.(h|c)$/); > > +}; > > + > > +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. > Done. > > + 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 > I hope to have correctly understood alignment rules... :) Raffaele P.S. The davinci-i2s patch is going to be sent in a couple of minutes. > Kevin > > > }; > > > > struct snd_soc_dai davinci_i2s_dai = { > > -- > > 1.7.0.4 > > > > _______________________________________________ > > Davinci-linux-open-source mailing list > > Davinci-linux-open-source@linux.davincidsp.com > > http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source > -- www.opensurf.it
_______________________________________________ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
