Re: [PATCH] ASoC: fsl_spdif: Add pm runtime function

2020-06-19 Thread Shengjiu Wang 
On Fri, Jun 19, 2020 at 1:51 PM Nicolin Chen  wrote:
>
> On Thu, Jun 18, 2020 at 07:55:34PM +0800, Shengjiu Wang wrote:
> > Add pm runtime support and move clock handling there.
> > Close the clocks at suspend to reduce the power consumption.
> >
> > fsl_spdif_suspend is replaced by pm_runtime_force_suspend.
> > fsl_spdif_resume is replaced by pm_runtime_force_resume.
> >
> > Signed-off-by: Shengjiu Wang 
>
> LGTM, yet some nits, please add my ack after fixing:
>
> Acked-by: Nicolin Chen 
>
> > @@ -495,25 +496,10 @@ static int fsl_spdif_startup(struct snd_pcm_substream 
> > *substream,
>
> >
> > -disable_txclk:
> > - for (i--; i >= 0; i--)
> > - clk_disable_unprepare(spdif_priv->txclk[i]);
> >  err:
> > - if (!IS_ERR(spdif_priv->spbaclk))
> > - clk_disable_unprepare(spdif_priv->spbaclk);
> > -err_spbaclk:
> > - clk_disable_unprepare(spdif_priv->coreclk);
> > -
> >   return ret;
>
> Only "return ret;" remains now. We could clean the goto away.
>
> > -static int fsl_spdif_resume(struct device *dev)
> > +static int fsl_spdif_runtime_resume(struct device *dev)
>
> > +disable_rx_clk:
> > + clk_disable_unprepare(spdif_priv->rxclk);
> > +disable_tx_clk:
> > +disable_spba_clk:
>
> Why have two duplicated ones? Could probably drop the 2nd one.

seems can drop one, will send an update.

best regards
wang shengjiu

Re: [PATCH] ASoC: fsl_spdif: Add pm runtime function

2020-06-18 Thread Nicolin Chen 
On Thu, Jun 18, 2020 at 07:55:34PM +0800, Shengjiu Wang wrote:
> Add pm runtime support and move clock handling there.
> Close the clocks at suspend to reduce the power consumption.
> 
> fsl_spdif_suspend is replaced by pm_runtime_force_suspend.
> fsl_spdif_resume is replaced by pm_runtime_force_resume.
> 
> Signed-off-by: Shengjiu Wang 

LGTM, yet some nits, please add my ack after fixing:

Acked-by: Nicolin Chen 

> @@ -495,25 +496,10 @@ static int fsl_spdif_startup(struct snd_pcm_substream 
> *substream,

>  
> -disable_txclk:
> - for (i--; i >= 0; i--)
> - clk_disable_unprepare(spdif_priv->txclk[i]);
>  err:
> - if (!IS_ERR(spdif_priv->spbaclk))
> - clk_disable_unprepare(spdif_priv->spbaclk);
> -err_spbaclk:
> - clk_disable_unprepare(spdif_priv->coreclk);
> -
>   return ret;

Only "return ret;" remains now. We could clean the goto away.

> -static int fsl_spdif_resume(struct device *dev)
> +static int fsl_spdif_runtime_resume(struct device *dev)

> +disable_rx_clk:
> + clk_disable_unprepare(spdif_priv->rxclk);
> +disable_tx_clk:
> +disable_spba_clk:

Why have two duplicated ones? Could probably drop the 2nd one.

[PATCH] ASoC: fsl_spdif: Add pm runtime function

2020-06-18 Thread Shengjiu Wang 
Add pm runtime support and move clock handling there.
Close the clocks at suspend to reduce the power consumption.

fsl_spdif_suspend is replaced by pm_runtime_force_suspend.
fsl_spdif_resume is replaced by pm_runtime_force_resume.

Signed-off-by: Shengjiu Wang 
---
 sound/soc/fsl/fsl_spdif.c | 113 ++
 1 file changed, 67 insertions(+), 46 deletions(-)

diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index 5bc0e4729341..46719fd2f1ec 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -495,25 +496,10 @@ static int fsl_spdif_startup(struct snd_pcm_substream 
*substream,
struct platform_device *pdev = spdif_priv->pdev;
struct regmap *regmap = spdif_priv->regmap;
u32 scr, mask;
-   int i;
int ret;
 
/* Reset module and interrupts only for first initialization */
if (!snd_soc_dai_active(cpu_dai)) {
-   ret = clk_prepare_enable(spdif_priv->coreclk);
-   if (ret) {
-   dev_err(>dev, "failed to enable core clock\n");
-   return ret;
-   }
-
-   if (!IS_ERR(spdif_priv->spbaclk)) {
-   ret = clk_prepare_enable(spdif_priv->spbaclk);
-   if (ret) {
-   dev_err(>dev, "failed to enable spba 
clock\n");
-   goto err_spbaclk;
-   }
-   }
-
ret = spdif_softreset(spdif_priv);
if (ret) {
dev_err(>dev, "failed to soft reset\n");
@@ -531,18 +517,10 @@ static int fsl_spdif_startup(struct snd_pcm_substream 
*substream,
mask = SCR_TXFIFO_AUTOSYNC_MASK | SCR_TXFIFO_CTRL_MASK |
SCR_TXSEL_MASK | SCR_USRC_SEL_MASK |
SCR_TXFIFO_FSEL_MASK;
-   for (i = 0; i < SPDIF_TXRATE_MAX; i++) {
-   ret = clk_prepare_enable(spdif_priv->txclk[i]);
-   if (ret)
-   goto disable_txclk;
-   }
} else {
scr = SCR_RXFIFO_FSEL_IF8 | SCR_RXFIFO_AUTOSYNC;
mask = SCR_RXFIFO_FSEL_MASK | SCR_RXFIFO_AUTOSYNC_MASK|
SCR_RXFIFO_CTL_MASK | SCR_RXFIFO_OFF_MASK;
-   ret = clk_prepare_enable(spdif_priv->rxclk);
-   if (ret)
-   goto err;
}
regmap_update_bits(regmap, REG_SPDIF_SCR, mask, scr);
 
@@ -551,15 +529,7 @@ static int fsl_spdif_startup(struct snd_pcm_substream 
*substream,
 
return 0;
 
-disable_txclk:
-   for (i--; i >= 0; i--)
-   clk_disable_unprepare(spdif_priv->txclk[i]);
 err:
-   if (!IS_ERR(spdif_priv->spbaclk))
-   clk_disable_unprepare(spdif_priv->spbaclk);
-err_spbaclk:
-   clk_disable_unprepare(spdif_priv->coreclk);
-
return ret;
 }
 
@@ -569,20 +539,17 @@ static void fsl_spdif_shutdown(struct snd_pcm_substream 
*substream,
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct fsl_spdif_priv *spdif_priv = 
snd_soc_dai_get_drvdata(asoc_rtd_to_cpu(rtd, 0));
struct regmap *regmap = spdif_priv->regmap;
-   u32 scr, mask, i;
+   u32 scr, mask;
 
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
scr = 0;
mask = SCR_TXFIFO_AUTOSYNC_MASK | SCR_TXFIFO_CTRL_MASK |
SCR_TXSEL_MASK | SCR_USRC_SEL_MASK |
SCR_TXFIFO_FSEL_MASK;
-   for (i = 0; i < SPDIF_TXRATE_MAX; i++)
-   clk_disable_unprepare(spdif_priv->txclk[i]);
} else {
scr = SCR_RXFIFO_OFF | SCR_RXFIFO_CTL_ZERO;
mask = SCR_RXFIFO_FSEL_MASK | SCR_RXFIFO_AUTOSYNC_MASK|
SCR_RXFIFO_CTL_MASK | SCR_RXFIFO_OFF_MASK;
-   clk_disable_unprepare(spdif_priv->rxclk);
}
regmap_update_bits(regmap, REG_SPDIF_SCR, mask, scr);
 
@@ -591,9 +558,6 @@ static void fsl_spdif_shutdown(struct snd_pcm_substream 
*substream,
spdif_intr_status_clear(spdif_priv);
regmap_update_bits(regmap, REG_SPDIF_SCR,
SCR_LOW_POWER, SCR_LOW_POWER);
-   if (!IS_ERR(spdif_priv->spbaclk))
-   clk_disable_unprepare(spdif_priv->spbaclk);
-   clk_disable_unprepare(spdif_priv->coreclk);
}
 }
 
@@ -1350,6 +1314,8 @@ static int fsl_spdif_probe(struct platform_device *pdev)
 
/* Register with ASoC */
dev_set_drvdata(>dev, spdif_priv);
+   pm_runtime_enable(>dev);
+   regcache_cache_only(spdif_priv->regmap, true);
 
ret = devm_snd_soc_register_component(>dev, _spdif_component,
  _priv->cpu_dai_drv, 1);
@@