| Message ID | 1398682498-24357-13-git-send-email-mpa@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 28, 2014 at 12:54:53PM +0200, Markus Pargmann wrote: > From: Sascha Hauer <s.hauer@pengutronix.de> > > The fsl_ssi driver uses the .set_sysclk callback to configure the > bitclock for master mode. This is unnecessary since the bitclock > is known in hw_params. This patch removes the .set_sysclk callback > and configures the bitclock from .hw_params. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- This isn't signed off either. Are we positive that none of the users set the bitclock to something non-default - there are devices out there which require an unusual bitclock (eg, where it's also used as the master clock and so needs overclocking)? Providing a default as this does is a definite win, but allowing it to be overridden may be needed sometimes (rarely fortunately). Leaving a stub set_sysclk() that just sets a variable and then using that value instead of calculating in hw_params() would get the best of both worlds.
Hi, On Tue, May 20, 2014 at 11:08:02PM +0100, Mark Brown wrote: > On Mon, Apr 28, 2014 at 12:54:53PM +0200, Markus Pargmann wrote: > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > The fsl_ssi driver uses the .set_sysclk callback to configure the > > bitclock for master mode. This is unnecessary since the bitclock > > is known in hw_params. This patch removes the .set_sysclk callback > > and configures the bitclock from .hw_params. > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > This isn't signed off either. Are we positive that none of the users > set the bitclock to something non-default - there are devices out there > which require an unusual bitclock (eg, where it's also used as the > master clock and so needs overclocking)? No, I don't know if there are actual users of this set_dai_sysclk. > > Providing a default as this does is a definite win, but allowing it to > be overridden may be needed sometimes (rarely fortunately). Leaving a > stub set_sysclk() that just sets a variable and then using that value > instead of calculating in hw_params() would get the best of both worlds. Okay, I will change it. Thanks, Markus
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index d8f9a5f..8ed724c 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -509,7 +509,7 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, } /** - * fsl_ssi_set_dai_sysclk - configure Digital Audio Interface bit clock + * fsl_ssi_set_bclk - configure Digital Audio Interface bit clock * * Note: This function can be only called when using SSI as DAI master * @@ -517,8 +517,8 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, * freq: Output BCLK frequency = samplerate * 32 (fixed) * channels * dir: SND_SOC_CLOCK_OUT -> TxBCLK, SND_SOC_CLOCK_IN -> RxBCLK. */ -static int fsl_ssi_set_dai_sysclk(struct snd_soc_dai *cpu_dai, - int clk_id, unsigned int freq, int dir) +static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream, + struct snd_soc_dai *cpu_dai, unsigned int freq) { struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(cpu_dai); struct ccsr_ssi __iomem *ssi = ssi_private->ssi; @@ -583,7 +583,7 @@ static int fsl_ssi_set_dai_sysclk(struct snd_soc_dai *cpu_dai, mask = CCSR_SSI_SxCCR_PM_MASK | CCSR_SSI_SxCCR_DIV2 | CCSR_SSI_SxCCR_PSR; - if (dir == SND_SOC_CLOCK_OUT || synchronous) + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK || synchronous) write_ssi_mask(&ssi->stccr, mask, stccr); else write_ssi_mask(&ssi->srccr, mask, stccr); @@ -627,6 +627,7 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream, snd_pcm_format_width(params_format(hw_params)); u32 wl = CCSR_SSI_SxCCR_WL(sample_size); int enabled = read_ssi(&ssi->scr) & CCSR_SSI_SCR_SSIEN; + int ret; /* * If we're in synchronous mode, and the SSI is already enabled, @@ -635,6 +636,14 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream, if (enabled && ssi_private->cpu_dai_drv.symmetric_rates) return 0; + if (fsl_ssi_is_i2s_master(ssi_private)) { + ret = fsl_ssi_set_bclk(substream, cpu_dai, + params_channels(hw_params) * 32 * + params_rate(hw_params)); + if (ret) + return ret; + } + /* * FIXME: The documentation says that SxCCR[WL] should not be * modified while the SSI is enabled. The only time this can @@ -923,7 +932,6 @@ static const struct snd_soc_dai_ops fsl_ssi_dai_ops = { .startup = fsl_ssi_startup, .hw_params = fsl_ssi_hw_params, .set_fmt = fsl_ssi_set_dai_fmt, - .set_sysclk = fsl_ssi_set_dai_sysclk, .set_tdm_slot = fsl_ssi_set_dai_tdm_slot, .trigger = fsl_ssi_trigger, };