| Message ID | 1397482548-28463-10-git-send-email-mpa@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 14, 2014 at 03:35:39PM +0200, Markus Pargmann wrote: > From: Sascha Hauer <s.hauer@pengutronix.de> > > Enable baudclk only when it is used. The baudclock is only needed in master > mode and only when thr driver is active, so move enabling to fsl_ssi_startup > and disabling to fsl_ssi_shutdown. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > sound/soc/fsl/fsl_ssi.c | 37 ++++++++++++++++++++++++++++++------- > 1 file changed, 30 insertions(+), 7 deletions(-) > > diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c > index f2255cb..d6d3f0a3 100644 > --- a/sound/soc/fsl/fsl_ssi.c > +++ b/sound/soc/fsl/fsl_ssi.c > @@ -236,6 +236,12 @@ static bool fsl_ssi_is_ac97(struct fsl_ssi_private *ssi_private) > return !!(ssi_private->dai_fmt & SND_SOC_DAIFMT_AC97); > } > > +static bool fsl_ssi_is_i2s_master(struct fsl_ssi_private *ssi_private) > +{ > + return (ssi_private->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) == > + SND_SOC_DAIFMT_CBS_CFS; > +} > + > /** > * fsl_ssi_isr: SSI interrupt handler > * > @@ -489,6 +495,7 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, > struct fsl_ssi_private *ssi_private = > snd_soc_dai_get_drvdata(rtd->cpu_dai); > unsigned long flags; > + int ret; > > if (!dai->active && !fsl_ssi_is_ac97(ssi_private)) { > spin_lock_irqsave(&ssi_private->baudclk_lock, flags); > @@ -496,6 +503,12 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, > spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); > } > > + if (fsl_ssi_is_i2s_master(ssi_private)) { > + ret = clk_prepare_enable(ssi_private->baudclk); > + if (ret) > + return ret; > + } > + > /* When using dual fifo mode, it is safer to ensure an even period > * size. If appearing to an odd number while DMA always starts its > * task from fifo0, fifo1 would be neglected at the end of each > @@ -508,6 +521,17 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, > return 0; > } > > +static void fsl_ssi_shutdown(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > + struct fsl_ssi_private *ssi_private = > + snd_soc_dai_get_drvdata(rtd->cpu_dai); > + > + if (fsl_ssi_is_i2s_master(ssi_private)) > + clk_disable_unprepare(ssi_private->baudclk); > +} > + > /** > * fsl_ssi_hw_params - program the sample size > * > @@ -576,6 +600,11 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) > > ssi_private->dai_fmt = fmt; > > + if (fsl_ssi_is_i2s_master(ssi_private) && IS_ERR(ssi_private->baudclk)) { > + dev_err(cpu_dai->dev, "no baudclk needed for master mode\n"); > + return -EINVAL; > + } > + I was wondering what if machine driver doesn't set fmt to master during its probe(), in another word -- before fsl_ssi_startup(), but leave that into its hw_params() via snd_soc_dai_set_fmt() which then would run after fsl_ssi_startup() while having no baud clock enabled in this case. A better solution may be to wrap clk_prepare_enable() and master mode clock dividing code into fsl_ssi_hw_params(), a bit like the ESAI driver even though it doesn't contain the clk_prepare_enable() part currently, and then to put clk_disable_unprepare() into hw_free() for symmetry. Any suggestion? Thank you, Nicolin > fsl_ssi_setup_reg_vals(ssi_private); > > scr = read_ssi(&ssi->scr) & ~(CCSR_SSI_SCR_SYN | CCSR_SSI_SCR_I2S_MODE_MASK); > @@ -910,6 +939,7 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai) > > static const struct snd_soc_dai_ops fsl_ssi_dai_ops = { > .startup = fsl_ssi_startup, > + .shutdown = fsl_ssi_shutdown, > .hw_params = fsl_ssi_hw_params, > .set_fmt = fsl_ssi_set_dai_fmt, > .set_sysclk = fsl_ssi_set_dai_sysclk, > @@ -1050,8 +1080,6 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, > if (IS_ERR(ssi_private->baudclk)) > dev_dbg(&pdev->dev, "could not get baud clock: %ld\n", > PTR_ERR(ssi_private->baudclk)); > - else > - clk_prepare_enable(ssi_private->baudclk); > > /* > * We have burstsize be "fifo_depth - 2" to match the SSI > @@ -1102,9 +1130,6 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, > return 0; > > error_pcm: > - if (!IS_ERR(ssi_private->baudclk)) > - clk_disable_unprepare(ssi_private->baudclk); > - > clk_disable_unprepare(ssi_private->clk); > > return ret; > @@ -1115,8 +1140,6 @@ static void fsl_ssi_imx_clean(struct platform_device *pdev, > { > if (!ssi_private->use_dma) > imx_pcm_fiq_exit(pdev); > - if (!IS_ERR(ssi_private->baudclk)) > - clk_disable_unprepare(ssi_private->baudclk); > clk_disable_unprepare(ssi_private->clk); > } > > -- > 1.9.1 > > >
Hi, On Mon, Apr 14, 2014 at 11:28:51PM +0800, Nicolin Chen wrote: > On Mon, Apr 14, 2014 at 03:35:39PM +0200, Markus Pargmann wrote: > > From: Sascha Hauer <s.hauer@pengutronix.de> > > > > Enable baudclk only when it is used. The baudclock is only needed in master > > mode and only when thr driver is active, so move enabling to fsl_ssi_startup > > and disabling to fsl_ssi_shutdown. > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > sound/soc/fsl/fsl_ssi.c | 37 ++++++++++++++++++++++++++++++------- > > 1 file changed, 30 insertions(+), 7 deletions(-) > > > > diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c > > index f2255cb..d6d3f0a3 100644 > > --- a/sound/soc/fsl/fsl_ssi.c > > +++ b/sound/soc/fsl/fsl_ssi.c > > @@ -236,6 +236,12 @@ static bool fsl_ssi_is_ac97(struct fsl_ssi_private *ssi_private) > > return !!(ssi_private->dai_fmt & SND_SOC_DAIFMT_AC97); > > } > > > > +static bool fsl_ssi_is_i2s_master(struct fsl_ssi_private *ssi_private) > > +{ > > + return (ssi_private->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) == > > + SND_SOC_DAIFMT_CBS_CFS; > > +} > > + > > /** > > * fsl_ssi_isr: SSI interrupt handler > > * > > @@ -489,6 +495,7 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, > > struct fsl_ssi_private *ssi_private = > > snd_soc_dai_get_drvdata(rtd->cpu_dai); > > unsigned long flags; > > + int ret; > > > > if (!dai->active && !fsl_ssi_is_ac97(ssi_private)) { > > spin_lock_irqsave(&ssi_private->baudclk_lock, flags); > > @@ -496,6 +503,12 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, > > spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); > > } > > > > + if (fsl_ssi_is_i2s_master(ssi_private)) { > > + ret = clk_prepare_enable(ssi_private->baudclk); > > + if (ret) > > + return ret; > > + } > > + > > /* When using dual fifo mode, it is safer to ensure an even period > > * size. If appearing to an odd number while DMA always starts its > > * task from fifo0, fifo1 would be neglected at the end of each > > @@ -508,6 +521,17 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, > > return 0; > > } > > > > +static void fsl_ssi_shutdown(struct snd_pcm_substream *substream, > > + struct snd_soc_dai *dai) > > +{ > > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > > + struct fsl_ssi_private *ssi_private = > > + snd_soc_dai_get_drvdata(rtd->cpu_dai); > > + > > + if (fsl_ssi_is_i2s_master(ssi_private)) > > + clk_disable_unprepare(ssi_private->baudclk); > > +} > > + > > /** > > * fsl_ssi_hw_params - program the sample size > > * > > @@ -576,6 +600,11 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) > > > > ssi_private->dai_fmt = fmt; > > > > + if (fsl_ssi_is_i2s_master(ssi_private) && IS_ERR(ssi_private->baudclk)) { > > + dev_err(cpu_dai->dev, "no baudclk needed for master mode\n"); > > + return -EINVAL; > > + } > > + > > I was wondering what if machine driver doesn't set fmt to master during > its probe(), in another word -- before fsl_ssi_startup(), but leave that > into its hw_params() via snd_soc_dai_set_fmt() which then would run after > fsl_ssi_startup() while having no baud clock enabled in this case. > > A better solution may be to wrap clk_prepare_enable() and master mode > clock dividing code into fsl_ssi_hw_params(), a bit like the ESAI driver > even though it doesn't contain the clk_prepare_enable() part currently, > and then to put clk_disable_unprepare() into hw_free() for symmetry. > > Any suggestion? Yes this is a problem. Although I am not really convinced of the concept of setting up the DAIs in hw_params, we should support it as there are some users. Is there always exactly one hw_free call for one hw_params call? There is a comment above the function: "Frees resources allocated by hw_params, can be called multiple times", so I am not sure if we can directly use clk_prepare_enable or if we need to remember the clock state? Thanks, Markus
Hi Markus, Please allow me to drop some content. On Wed, Apr 16, 2014 at 09:27:38AM +0200, Markus Pargmann wrote: > On Mon, Apr 14, 2014 at 11:28:51PM +0800, Nicolin Chen wrote: > > On Mon, Apr 14, 2014 at 03:35:39PM +0200, Markus Pargmann wrote: > > > +static bool fsl_ssi_is_i2s_master(struct fsl_ssi_private *ssi_private) > > > +{ > > > + return (ssi_private->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) == > > > + SND_SOC_DAIFMT_CBS_CFS; > > > +} > > > @@ -496,6 +503,12 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, > > > spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); > > > } > > > > > > + if (fsl_ssi_is_i2s_master(ssi_private)) { > > > + ret = clk_prepare_enable(ssi_private->baudclk); > > > + if (ret) > > > + return ret; > > > + } > > > +static void fsl_ssi_shutdown(struct snd_pcm_substream *substream, > > > + struct snd_soc_dai *dai) > > > +{ > > > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > > > + struct fsl_ssi_private *ssi_private = > > > + snd_soc_dai_get_drvdata(rtd->cpu_dai); > > > + > > > + if (fsl_ssi_is_i2s_master(ssi_private)) > > > + clk_disable_unprepare(ssi_private->baudclk); > > > +} > > > @@ -576,6 +600,11 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) > > > > > > ssi_private->dai_fmt = fmt; > > > > > > + if (fsl_ssi_is_i2s_master(ssi_private) && IS_ERR(ssi_private->baudclk)) { > > > + dev_err(cpu_dai->dev, "no baudclk needed for master mode\n"); > > > + return -EINVAL; > > > + } > > > + > > > > I was wondering what if machine driver doesn't set fmt to master during > > its probe(), in another word -- before fsl_ssi_startup(), but leave that > > into its hw_params() via snd_soc_dai_set_fmt() which then would run after > > fsl_ssi_startup() while having no baud clock enabled in this case. > > > > A better solution may be to wrap clk_prepare_enable() and master mode > > clock dividing code into fsl_ssi_hw_params(), a bit like the ESAI driver > > even though it doesn't contain the clk_prepare_enable() part currently, > > and then to put clk_disable_unprepare() into hw_free() for symmetry. > > > > Any suggestion? > > Yes this is a problem. Although I am not really convinced of the concept > of setting up the DAIs in hw_params, we should support it as there are > some users. I think it might be an old fashion way. I saw quite a lot machine drivers doing the DAI format setting in its hw_params(). Even in the current tree we can also grep some out by using: find ./sound/soc/ -name "*.c" |xargs grep "snd_soc_dai_set_fmt" So it should be plausible considering there might be some special cases, using CBS_CFS for 44.1KHz groups and CBM_CFM for 48KHz groups for example. > Is there always exactly one hw_free call for one hw_params call? There > is a comment above the function: > "Frees resources allocated by hw_params, can be called multiple times", That's a good question. IIRC, snd_pcm_release_substream() would call its hw_free() right before calling snd_pcm_close(). So there would be at least twice for hw_free()'s execution. > so I am not sure if we can directly use clk_prepare_enable or if we need > to remember the clock state? We need to if applying that. Or put them into trigger() as an alternative approach even if it sounds weird to me. Thanks, Nicolin
On Wed, Apr 16, 2014 at 10:43:31AM +0200, Markus Pargmann wrote: > Hi Nicolin, > > On Wed, Apr 16, 2014 at 04:08:29PM +0800, Nicolin Chen wrote: > > Hi Markus, > > > > Please allow me to drop some content. > > > > On Wed, Apr 16, 2014 at 09:27:38AM +0200, Markus Pargmann wrote: > > > On Mon, Apr 14, 2014 at 11:28:51PM +0800, Nicolin Chen wrote: > > > > On Mon, Apr 14, 2014 at 03:35:39PM +0200, Markus Pargmann wrote: > > > > > +static bool fsl_ssi_is_i2s_master(struct fsl_ssi_private *ssi_private) > > > > > +{ > > > > > + return (ssi_private->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) == > > > > > + SND_SOC_DAIFMT_CBS_CFS; > > > > > +} > > > > > > > @@ -496,6 +503,12 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, > > > > > spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); > > > > > } > > > > > > > > > > + if (fsl_ssi_is_i2s_master(ssi_private)) { > > > > > + ret = clk_prepare_enable(ssi_private->baudclk); > > > > > + if (ret) > > > > > + return ret; > > > > > + } > > > > > > > +static void fsl_ssi_shutdown(struct snd_pcm_substream *substream, > > > > > + struct snd_soc_dai *dai) > > > > > +{ > > > > > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > > > > > + struct fsl_ssi_private *ssi_private = > > > > > + snd_soc_dai_get_drvdata(rtd->cpu_dai); > > > > > + > > > > > + if (fsl_ssi_is_i2s_master(ssi_private)) > > > > > + clk_disable_unprepare(ssi_private->baudclk); > > > > > +} > > > > > > > @@ -576,6 +600,11 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) > > > > > > > > > > ssi_private->dai_fmt = fmt; > > > > > > > > > > + if (fsl_ssi_is_i2s_master(ssi_private) && IS_ERR(ssi_private->baudclk)) { > > > > > + dev_err(cpu_dai->dev, "no baudclk needed for master mode\n"); > > > > > + return -EINVAL; > > > > > + } > > > > > + > > > > > > > > I was wondering what if machine driver doesn't set fmt to master during > > > > its probe(), in another word -- before fsl_ssi_startup(), but leave that > > > > into its hw_params() via snd_soc_dai_set_fmt() which then would run after > > > > fsl_ssi_startup() while having no baud clock enabled in this case. > > > > > > > > A better solution may be to wrap clk_prepare_enable() and master mode > > > > clock dividing code into fsl_ssi_hw_params(), a bit like the ESAI driver > > > > even though it doesn't contain the clk_prepare_enable() part currently, > > > > and then to put clk_disable_unprepare() into hw_free() for symmetry. > > > > > > > > Any suggestion? > > > > > > Yes this is a problem. Although I am not really convinced of the concept > > > of setting up the DAIs in hw_params, we should support it as there are > > > some users. > > > > I think it might be an old fashion way. I saw quite a lot machine drivers > > doing the DAI format setting in its hw_params(). Even in the current tree > > we can also grep some out by using: > > > > find ./sound/soc/ -name "*.c" |xargs grep "snd_soc_dai_set_fmt" > > > > So it should be plausible considering there might be some special cases, > > using CBS_CFS for 44.1KHz groups and CBM_CFM for 48KHz groups for example. > > > > > Is there always exactly one hw_free call for one hw_params call? There > > > is a comment above the function: > > > "Frees resources allocated by hw_params, can be called multiple times", > > > > That's a good question. IIRC, snd_pcm_release_substream() would call its > > hw_free() right before calling snd_pcm_close(). So there would be at least > > twice for hw_free()'s execution. > > > > > so I am not sure if we can directly use clk_prepare_enable or if we need > > > to remember the clock state? > > > > We need to if applying that. Or put them into trigger() as an alternative > > approach even if it sounds weird to me. > > Unfortunately trigger() is not an option, it also may be called multiple > times for one command due to strange error handling in the layers above. > For example a failing START command in the DMA driver may lead to a > STOP command in the fsl-ssi driver without a previous START command. Nice judgement. Okay, just forget about trigger() then. > So it seems the only option is to save the clock state? I will move it > to hw_params() and hw_free(). I think we might wait other's opinion on this topic or try to send an extra patch for it meanwhile you can just drop the single patch from this series so that the other patches will be easily applied since they are really useful and do fix a few problems. Cheers, Nicolin > > Thanks, > > Markus > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Nicolin, On Wed, Apr 16, 2014 at 04:08:29PM +0800, Nicolin Chen wrote: > Hi Markus, > > Please allow me to drop some content. > > On Wed, Apr 16, 2014 at 09:27:38AM +0200, Markus Pargmann wrote: > > On Mon, Apr 14, 2014 at 11:28:51PM +0800, Nicolin Chen wrote: > > > On Mon, Apr 14, 2014 at 03:35:39PM +0200, Markus Pargmann wrote: > > > > +static bool fsl_ssi_is_i2s_master(struct fsl_ssi_private *ssi_private) > > > > +{ > > > > + return (ssi_private->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) == > > > > + SND_SOC_DAIFMT_CBS_CFS; > > > > +} > > > > > @@ -496,6 +503,12 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, > > > > spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); > > > > } > > > > > > > > + if (fsl_ssi_is_i2s_master(ssi_private)) { > > > > + ret = clk_prepare_enable(ssi_private->baudclk); > > > > + if (ret) > > > > + return ret; > > > > + } > > > > > +static void fsl_ssi_shutdown(struct snd_pcm_substream *substream, > > > > + struct snd_soc_dai *dai) > > > > +{ > > > > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > > > > + struct fsl_ssi_private *ssi_private = > > > > + snd_soc_dai_get_drvdata(rtd->cpu_dai); > > > > + > > > > + if (fsl_ssi_is_i2s_master(ssi_private)) > > > > + clk_disable_unprepare(ssi_private->baudclk); > > > > +} > > > > > @@ -576,6 +600,11 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) > > > > > > > > ssi_private->dai_fmt = fmt; > > > > > > > > + if (fsl_ssi_is_i2s_master(ssi_private) && IS_ERR(ssi_private->baudclk)) { > > > > + dev_err(cpu_dai->dev, "no baudclk needed for master mode\n"); > > > > + return -EINVAL; > > > > + } > > > > + > > > > > > I was wondering what if machine driver doesn't set fmt to master during > > > its probe(), in another word -- before fsl_ssi_startup(), but leave that > > > into its hw_params() via snd_soc_dai_set_fmt() which then would run after > > > fsl_ssi_startup() while having no baud clock enabled in this case. > > > > > > A better solution may be to wrap clk_prepare_enable() and master mode > > > clock dividing code into fsl_ssi_hw_params(), a bit like the ESAI driver > > > even though it doesn't contain the clk_prepare_enable() part currently, > > > and then to put clk_disable_unprepare() into hw_free() for symmetry. > > > > > > Any suggestion? > > > > Yes this is a problem. Although I am not really convinced of the concept > > of setting up the DAIs in hw_params, we should support it as there are > > some users. > > I think it might be an old fashion way. I saw quite a lot machine drivers > doing the DAI format setting in its hw_params(). Even in the current tree > we can also grep some out by using: > > find ./sound/soc/ -name "*.c" |xargs grep "snd_soc_dai_set_fmt" > > So it should be plausible considering there might be some special cases, > using CBS_CFS for 44.1KHz groups and CBM_CFM for 48KHz groups for example. > > > Is there always exactly one hw_free call for one hw_params call? There > > is a comment above the function: > > "Frees resources allocated by hw_params, can be called multiple times", > > That's a good question. IIRC, snd_pcm_release_substream() would call its > hw_free() right before calling snd_pcm_close(). So there would be at least > twice for hw_free()'s execution. > > > so I am not sure if we can directly use clk_prepare_enable or if we need > > to remember the clock state? > > We need to if applying that. Or put them into trigger() as an alternative > approach even if it sounds weird to me. Unfortunately trigger() is not an option, it also may be called multiple times for one command due to strange error handling in the layers above. For example a failing START command in the DMA driver may lead to a STOP command in the fsl-ssi driver without a previous START command. So it seems the only option is to save the clock state? I will move it to hw_params() and hw_free(). Thanks, Markus
On Wed, Apr 16, 2014 at 04:08:29PM +0800, Nicolin Chen wrote: > On Wed, Apr 16, 2014 at 09:27:38AM +0200, Markus Pargmann wrote: > > Is there always exactly one hw_free call for one hw_params call? There > > is a comment above the function: > > "Frees resources allocated by hw_params, can be called multiple times", > That's a good question. IIRC, snd_pcm_release_substream() would call its > hw_free() right before calling snd_pcm_close(). So there would be at least > twice for hw_free()'s execution. There may be any number of hw_params() calls.
Markus Pargmann wrote:
> Is there always exactly one hw_free call for one hw_params call?
I don't think so. It's been a while since I've worked on this code, but
I do remember noticing that hw_params could be called multiple times,
whereas hw_free was called only once. This made a big impact on my
design of the code.
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index f2255cb..d6d3f0a3 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -236,6 +236,12 @@ static bool fsl_ssi_is_ac97(struct fsl_ssi_private *ssi_private) return !!(ssi_private->dai_fmt & SND_SOC_DAIFMT_AC97); } +static bool fsl_ssi_is_i2s_master(struct fsl_ssi_private *ssi_private) +{ + return (ssi_private->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) == + SND_SOC_DAIFMT_CBS_CFS; +} + /** * fsl_ssi_isr: SSI interrupt handler * @@ -489,6 +495,7 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(rtd->cpu_dai); unsigned long flags; + int ret; if (!dai->active && !fsl_ssi_is_ac97(ssi_private)) { spin_lock_irqsave(&ssi_private->baudclk_lock, flags); @@ -496,6 +503,12 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, spin_unlock_irqrestore(&ssi_private->baudclk_lock, flags); } + if (fsl_ssi_is_i2s_master(ssi_private)) { + ret = clk_prepare_enable(ssi_private->baudclk); + if (ret) + return ret; + } + /* When using dual fifo mode, it is safer to ensure an even period * size. If appearing to an odd number while DMA always starts its * task from fifo0, fifo1 would be neglected at the end of each @@ -508,6 +521,17 @@ static int fsl_ssi_startup(struct snd_pcm_substream *substream, return 0; } +static void fsl_ssi_shutdown(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct fsl_ssi_private *ssi_private = + snd_soc_dai_get_drvdata(rtd->cpu_dai); + + if (fsl_ssi_is_i2s_master(ssi_private)) + clk_disable_unprepare(ssi_private->baudclk); +} + /** * fsl_ssi_hw_params - program the sample size * @@ -576,6 +600,11 @@ static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt) ssi_private->dai_fmt = fmt; + if (fsl_ssi_is_i2s_master(ssi_private) && IS_ERR(ssi_private->baudclk)) { + dev_err(cpu_dai->dev, "no baudclk needed for master mode\n"); + return -EINVAL; + } + fsl_ssi_setup_reg_vals(ssi_private); scr = read_ssi(&ssi->scr) & ~(CCSR_SSI_SCR_SYN | CCSR_SSI_SCR_I2S_MODE_MASK); @@ -910,6 +939,7 @@ static int fsl_ssi_dai_probe(struct snd_soc_dai *dai) static const struct snd_soc_dai_ops fsl_ssi_dai_ops = { .startup = fsl_ssi_startup, + .shutdown = fsl_ssi_shutdown, .hw_params = fsl_ssi_hw_params, .set_fmt = fsl_ssi_set_dai_fmt, .set_sysclk = fsl_ssi_set_dai_sysclk, @@ -1050,8 +1080,6 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, if (IS_ERR(ssi_private->baudclk)) dev_dbg(&pdev->dev, "could not get baud clock: %ld\n", PTR_ERR(ssi_private->baudclk)); - else - clk_prepare_enable(ssi_private->baudclk); /* * We have burstsize be "fifo_depth - 2" to match the SSI @@ -1102,9 +1130,6 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, return 0; error_pcm: - if (!IS_ERR(ssi_private->baudclk)) - clk_disable_unprepare(ssi_private->baudclk); - clk_disable_unprepare(ssi_private->clk); return ret; @@ -1115,8 +1140,6 @@ static void fsl_ssi_imx_clean(struct platform_device *pdev, { if (!ssi_private->use_dma) imx_pcm_fiq_exit(pdev); - if (!IS_ERR(ssi_private->baudclk)) - clk_disable_unprepare(ssi_private->baudclk); clk_disable_unprepare(ssi_private->clk); }