[PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume

[PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume

[Shengjiu Wang]

With dedicated power domain for asrc, power can be disabled after
probe and pm runtime suspend, then the value of all registers need to
be restored in pm runtime resume. So we can merge suspend/resume function
to runtime_suspend/resume function and enable regcache only in end of
probe.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/fsl_asrc.c | 70 ++++++++++++++++------------------------
 1 file changed, 27 insertions(+), 43 deletions(-)

diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 432936039de4..3ebbe15ac378 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -1100,6 +1100,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, asrc);
 	pm_runtime_enable(&pdev->dev);
 	spin_lock_init(&asrc->lock);
+	regcache_cache_only(asrc->regmap, true);
 
 	ret = devm_snd_soc_register_component(&pdev->dev, &fsl_asrc_component,
 					      &fsl_asrc_dai, 1);
@@ -1117,6 +1118,7 @@ static int fsl_asrc_runtime_resume(struct device *dev)
 	struct fsl_asrc *asrc = dev_get_drvdata(dev);
 	struct fsl_asrc_priv *asrc_priv = asrc->private;
 	int i, ret;
+	u32 asrctr;
 
 	ret = clk_prepare_enable(asrc->mem_clk);
 	if (ret)
@@ -1135,6 +1137,24 @@ static int fsl_asrc_runtime_resume(struct device *dev)
 			goto disable_asrck_clk;
 	}
 
+	/* Stop all pairs provisionally */
+	regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
+	regmap_update_bits(asrc->regmap, REG_ASRCTR,
+			   ASRCTR_ASRCEi_ALL_MASK, 0);
+
+	/* Restore all registers */
+	regcache_cache_only(asrc->regmap, false);
+	regcache_mark_dirty(asrc->regmap);
+	regcache_sync(asrc->regmap);
+
+	regmap_update_bits(asrc->regmap, REG_ASRCFG,
+			   ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
+			   ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
+
+	/* Restart enabled pairs */
+	regmap_update_bits(asrc->regmap, REG_ASRCTR,
+			   ASRCTR_ASRCEi_ALL_MASK, asrctr);
+
 	return 0;
 
 disable_asrck_clk:
@@ -1155,6 +1175,11 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
 	struct fsl_asrc_priv *asrc_priv = asrc->private;
 	int i;
 
+	regmap_read(asrc->regmap, REG_ASRCFG,
+		    &asrc_priv->regcache_cfg);
+
+	regcache_cache_only(asrc->regmap, true);
+
 	for (i = 0; i < ASRC_CLK_MAX_NUM; i++)
 		clk_disable_unprepare(asrc_priv->asrck_clk[i]);
 	if (!IS_ERR(asrc->spba_clk))
@@ -1166,51 +1191,10 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
 }
 #endif /* CONFIG_PM */
 
-#ifdef CONFIG_PM_SLEEP
-static int fsl_asrc_suspend(struct device *dev)
-{
-	struct fsl_asrc *asrc = dev_get_drvdata(dev);
-	struct fsl_asrc_priv *asrc_priv = asrc->private;
-
-	regmap_read(asrc->regmap, REG_ASRCFG,
-		    &asrc_priv->regcache_cfg);
-
-	regcache_cache_only(asrc->regmap, true);
-	regcache_mark_dirty(asrc->regmap);
-
-	return 0;
-}
-
-static int fsl_asrc_resume(struct device *dev)
-{
-	struct fsl_asrc *asrc = dev_get_drvdata(dev);
-	struct fsl_asrc_priv *asrc_priv = asrc->private;
-	u32 asrctr;
-
-	/* Stop all pairs provisionally */
-	regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
-	regmap_update_bits(asrc->regmap, REG_ASRCTR,
-			   ASRCTR_ASRCEi_ALL_MASK, 0);
-
-	/* Restore all registers */
-	regcache_cache_only(asrc->regmap, false);
-	regcache_sync(asrc->regmap);
-
-	regmap_update_bits(asrc->regmap, REG_ASRCFG,
-			   ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
-			   ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
-
-	/* Restart enabled pairs */
-	regmap_update_bits(asrc->regmap, REG_ASRCTR,
-			   ASRCTR_ASRCEi_ALL_MASK, asrctr);
-
-	return 0;
-}
-#endif /* CONFIG_PM_SLEEP */
-
 static const struct dev_pm_ops fsl_asrc_pm = {
 	SET_RUNTIME_PM_OPS(fsl_asrc_runtime_suspend, fsl_asrc_runtime_resume, NULL)
-	SET_SYSTEM_SLEEP_PM_OPS(fsl_asrc_suspend, fsl_asrc_resume)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
 };
 
 static const struct fsl_asrc_soc_data fsl_asrc_imx35_data = {
-- 
2.21.0

Re: [PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume

[Nicolin Chen]

On Fri, May 22, 2020 at 05:57:24PM +0800, Shengjiu Wang wrote:
> With dedicated power domain for asrc, power can be disabled after
> probe and pm runtime suspend, then the value of all registers need to
> be restored in pm runtime resume. So we can merge suspend/resume function
> to runtime_suspend/resume function and enable regcache only in end of
> probe.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
>  sound/soc/fsl/fsl_asrc.c | 70 ++++++++++++++++------------------------
>  1 file changed, 27 insertions(+), 43 deletions(-)
> 
> diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> index 432936039de4..3ebbe15ac378 100644
> --- a/sound/soc/fsl/fsl_asrc.c
> +++ b/sound/soc/fsl/fsl_asrc.c
> @@ -1100,6 +1100,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, asrc);
>  	pm_runtime_enable(&pdev->dev);
>  	spin_lock_init(&asrc->lock);
> +	regcache_cache_only(asrc->regmap, true);
>  
>  	ret = devm_snd_soc_register_component(&pdev->dev, &fsl_asrc_component,
>  					      &fsl_asrc_dai, 1);
> @@ -1117,6 +1118,7 @@ static int fsl_asrc_runtime_resume(struct device *dev)
>  	struct fsl_asrc *asrc = dev_get_drvdata(dev);
>  	struct fsl_asrc_priv *asrc_priv = asrc->private;
>  	int i, ret;
> +	u32 asrctr;
>  
>  	ret = clk_prepare_enable(asrc->mem_clk);
>  	if (ret)
> @@ -1135,6 +1137,24 @@ static int fsl_asrc_runtime_resume(struct device *dev)
>  			goto disable_asrck_clk;
>  	}
>  
> +	/* Stop all pairs provisionally */
> +	regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
> +	regmap_update_bits(asrc->regmap, REG_ASRCTR,
> +			   ASRCTR_ASRCEi_ALL_MASK, 0);
> +
> +	/* Restore all registers */
> +	regcache_cache_only(asrc->regmap, false);
> +	regcache_mark_dirty(asrc->regmap);


I see you doing regcache_mark_dirty() in the resume() now,
being different from previously doing in suspend()?

Thanks
Nic


> +	regcache_sync(asrc->regmap);
> +
> +	regmap_update_bits(asrc->regmap, REG_ASRCFG,
> +			   ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
> +			   ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
> +
> +	/* Restart enabled pairs */
> +	regmap_update_bits(asrc->regmap, REG_ASRCTR,
> +			   ASRCTR_ASRCEi_ALL_MASK, asrctr);
> +
>  	return 0;
>  
>  disable_asrck_clk:
> @@ -1155,6 +1175,11 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
>  	struct fsl_asrc_priv *asrc_priv = asrc->private;
>  	int i;
>  
> +	regmap_read(asrc->regmap, REG_ASRCFG,
> +		    &asrc_priv->regcache_cfg);
> +
> +	regcache_cache_only(asrc->regmap, true);
> +
>  	for (i = 0; i < ASRC_CLK_MAX_NUM; i++)
>  		clk_disable_unprepare(asrc_priv->asrck_clk[i]);
>  	if (!IS_ERR(asrc->spba_clk))
> @@ -1166,51 +1191,10 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
>  }
>  #endif /* CONFIG_PM */
>  
> -#ifdef CONFIG_PM_SLEEP
> -static int fsl_asrc_suspend(struct device *dev)
> -{
> -	struct fsl_asrc *asrc = dev_get_drvdata(dev);
> -	struct fsl_asrc_priv *asrc_priv = asrc->private;
> -
> -	regmap_read(asrc->regmap, REG_ASRCFG,
> -		    &asrc_priv->regcache_cfg);
> -
> -	regcache_cache_only(asrc->regmap, true);
> -	regcache_mark_dirty(asrc->regmap);
> -
> -	return 0;
> -}
> -
> -static int fsl_asrc_resume(struct device *dev)
> -{
> -	struct fsl_asrc *asrc = dev_get_drvdata(dev);
> -	struct fsl_asrc_priv *asrc_priv = asrc->private;
> -	u32 asrctr;
> -
> -	/* Stop all pairs provisionally */
> -	regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
> -	regmap_update_bits(asrc->regmap, REG_ASRCTR,
> -			   ASRCTR_ASRCEi_ALL_MASK, 0);
> -
> -	/* Restore all registers */
> -	regcache_cache_only(asrc->regmap, false);
> -	regcache_sync(asrc->regmap);
> -
> -	regmap_update_bits(asrc->regmap, REG_ASRCFG,
> -			   ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
> -			   ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
> -
> -	/* Restart enabled pairs */
> -	regmap_update_bits(asrc->regmap, REG_ASRCTR,
> -			   ASRCTR_ASRCEi_ALL_MASK, asrctr);
> -
> -	return 0;
> -}
> -#endif /* CONFIG_PM_SLEEP */
> -
>  static const struct dev_pm_ops fsl_asrc_pm = {
>  	SET_RUNTIME_PM_OPS(fsl_asrc_runtime_suspend, fsl_asrc_runtime_resume, NULL)
> -	SET_SYSTEM_SLEEP_PM_OPS(fsl_asrc_suspend, fsl_asrc_resume)
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				pm_runtime_force_resume)
>  };
>  
>  static const struct fsl_asrc_soc_data fsl_asrc_imx35_data = {
> -- 
> 2.21.0
> 

Re: [PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume

[Nicolin Chen]

On Fri, May 22, 2020 at 05:57:24PM +0800, Shengjiu Wang wrote:
> With dedicated power domain for asrc, power can be disabled after
> probe and pm runtime suspend, then the value of all registers need to
> be restored in pm runtime resume. So we can merge suspend/resume function
> to runtime_suspend/resume function and enable regcache only in end of
> probe.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
>  sound/soc/fsl/fsl_asrc.c | 70 ++++++++++++++++------------------------
>  1 file changed, 27 insertions(+), 43 deletions(-)
> 
> diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> index 432936039de4..3ebbe15ac378 100644
> --- a/sound/soc/fsl/fsl_asrc.c
> +++ b/sound/soc/fsl/fsl_asrc.c
> @@ -1100,6 +1100,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, asrc);
>  	pm_runtime_enable(&pdev->dev);
>  	spin_lock_init(&asrc->lock);
> +	regcache_cache_only(asrc->regmap, true);
>  
>  	ret = devm_snd_soc_register_component(&pdev->dev, &fsl_asrc_component,
>  					      &fsl_asrc_dai, 1);
> @@ -1117,6 +1118,7 @@ static int fsl_asrc_runtime_resume(struct device *dev)
>  	struct fsl_asrc *asrc = dev_get_drvdata(dev);
>  	struct fsl_asrc_priv *asrc_priv = asrc->private;
>  	int i, ret;
> +	u32 asrctr;
>  
>  	ret = clk_prepare_enable(asrc->mem_clk);
>  	if (ret)
> @@ -1135,6 +1137,24 @@ static int fsl_asrc_runtime_resume(struct device *dev)
>  			goto disable_asrck_clk;
>  	}
>  
> +	/* Stop all pairs provisionally */
> +	regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
> +	regmap_update_bits(asrc->regmap, REG_ASRCTR,
> +			   ASRCTR_ASRCEi_ALL_MASK, 0);
> +
> +	/* Restore all registers */
> +	regcache_cache_only(asrc->regmap, false);
> +	regcache_mark_dirty(asrc->regmap);


I see you doing regcache_mark_dirty() in the resume() now,
being different from previously doing in suspend()?

Thanks
Nic


> +	regcache_sync(asrc->regmap);
> +
> +	regmap_update_bits(asrc->regmap, REG_ASRCFG,
> +			   ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
> +			   ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
> +
> +	/* Restart enabled pairs */
> +	regmap_update_bits(asrc->regmap, REG_ASRCTR,
> +			   ASRCTR_ASRCEi_ALL_MASK, asrctr);
> +
>  	return 0;
>  
>  disable_asrck_clk:
> @@ -1155,6 +1175,11 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
>  	struct fsl_asrc_priv *asrc_priv = asrc->private;
>  	int i;
>  
> +	regmap_read(asrc->regmap, REG_ASRCFG,
> +		    &asrc_priv->regcache_cfg);
> +
> +	regcache_cache_only(asrc->regmap, true);
> +
>  	for (i = 0; i < ASRC_CLK_MAX_NUM; i++)
>  		clk_disable_unprepare(asrc_priv->asrck_clk[i]);
>  	if (!IS_ERR(asrc->spba_clk))
> @@ -1166,51 +1191,10 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
>  }
>  #endif /* CONFIG_PM */
>  
> -#ifdef CONFIG_PM_SLEEP
> -static int fsl_asrc_suspend(struct device *dev)
> -{
> -	struct fsl_asrc *asrc = dev_get_drvdata(dev);
> -	struct fsl_asrc_priv *asrc_priv = asrc->private;
> -
> -	regmap_read(asrc->regmap, REG_ASRCFG,
> -		    &asrc_priv->regcache_cfg);
> -
> -	regcache_cache_only(asrc->regmap, true);
> -	regcache_mark_dirty(asrc->regmap);
> -
> -	return 0;
> -}
> -
> -static int fsl_asrc_resume(struct device *dev)
> -{
> -	struct fsl_asrc *asrc = dev_get_drvdata(dev);
> -	struct fsl_asrc_priv *asrc_priv = asrc->private;
> -	u32 asrctr;
> -
> -	/* Stop all pairs provisionally */
> -	regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
> -	regmap_update_bits(asrc->regmap, REG_ASRCTR,
> -			   ASRCTR_ASRCEi_ALL_MASK, 0);
> -
> -	/* Restore all registers */
> -	regcache_cache_only(asrc->regmap, false);
> -	regcache_sync(asrc->regmap);
> -
> -	regmap_update_bits(asrc->regmap, REG_ASRCFG,
> -			   ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
> -			   ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
> -
> -	/* Restart enabled pairs */
> -	regmap_update_bits(asrc->regmap, REG_ASRCTR,
> -			   ASRCTR_ASRCEi_ALL_MASK, asrctr);
> -
> -	return 0;
> -}
> -#endif /* CONFIG_PM_SLEEP */
> -
>  static const struct dev_pm_ops fsl_asrc_pm = {
>  	SET_RUNTIME_PM_OPS(fsl_asrc_runtime_suspend, fsl_asrc_runtime_resume, NULL)
> -	SET_SYSTEM_SLEEP_PM_OPS(fsl_asrc_suspend, fsl_asrc_resume)
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				pm_runtime_force_resume)
>  };
>  
>  static const struct fsl_asrc_soc_data fsl_asrc_imx35_data = {
> -- 
> 2.21.0
> 

Re: [PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume

[Nicolin Chen]

On Fri, May 22, 2020 at 05:57:24PM +0800, Shengjiu Wang wrote:
> With dedicated power domain for asrc, power can be disabled after
> probe and pm runtime suspend, then the value of all registers need to
> be restored in pm runtime resume. So we can merge suspend/resume function
> to runtime_suspend/resume function and enable regcache only in end of
> probe.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
>  sound/soc/fsl/fsl_asrc.c | 70 ++++++++++++++++------------------------
>  1 file changed, 27 insertions(+), 43 deletions(-)
> 
> diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> index 432936039de4..3ebbe15ac378 100644
> --- a/sound/soc/fsl/fsl_asrc.c
> +++ b/sound/soc/fsl/fsl_asrc.c
> @@ -1100,6 +1100,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, asrc);
>  	pm_runtime_enable(&pdev->dev);
>  	spin_lock_init(&asrc->lock);
> +	regcache_cache_only(asrc->regmap, true);
>  
>  	ret = devm_snd_soc_register_component(&pdev->dev, &fsl_asrc_component,
>  					      &fsl_asrc_dai, 1);
> @@ -1117,6 +1118,7 @@ static int fsl_asrc_runtime_resume(struct device *dev)
>  	struct fsl_asrc *asrc = dev_get_drvdata(dev);
>  	struct fsl_asrc_priv *asrc_priv = asrc->private;
>  	int i, ret;
> +	u32 asrctr;
>  
>  	ret = clk_prepare_enable(asrc->mem_clk);
>  	if (ret)
> @@ -1135,6 +1137,24 @@ static int fsl_asrc_runtime_resume(struct device *dev)
>  			goto disable_asrck_clk;
>  	}
>  
> +	/* Stop all pairs provisionally */
> +	regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
> +	regmap_update_bits(asrc->regmap, REG_ASRCTR,
> +			   ASRCTR_ASRCEi_ALL_MASK, 0);
> +
> +	/* Restore all registers */
> +	regcache_cache_only(asrc->regmap, false);
> +	regcache_mark_dirty(asrc->regmap);


I see you doing regcache_mark_dirty() in the resume() now,
being different from previously doing in suspend()?

Thanks
Nic


> +	regcache_sync(asrc->regmap);
> +
> +	regmap_update_bits(asrc->regmap, REG_ASRCFG,
> +			   ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
> +			   ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
> +
> +	/* Restart enabled pairs */
> +	regmap_update_bits(asrc->regmap, REG_ASRCTR,
> +			   ASRCTR_ASRCEi_ALL_MASK, asrctr);
> +
>  	return 0;
>  
>  disable_asrck_clk:
> @@ -1155,6 +1175,11 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
>  	struct fsl_asrc_priv *asrc_priv = asrc->private;
>  	int i;
>  
> +	regmap_read(asrc->regmap, REG_ASRCFG,
> +		    &asrc_priv->regcache_cfg);
> +
> +	regcache_cache_only(asrc->regmap, true);
> +
>  	for (i = 0; i < ASRC_CLK_MAX_NUM; i++)
>  		clk_disable_unprepare(asrc_priv->asrck_clk[i]);
>  	if (!IS_ERR(asrc->spba_clk))
> @@ -1166,51 +1191,10 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
>  }
>  #endif /* CONFIG_PM */
>  
> -#ifdef CONFIG_PM_SLEEP
> -static int fsl_asrc_suspend(struct device *dev)
> -{
> -	struct fsl_asrc *asrc = dev_get_drvdata(dev);
> -	struct fsl_asrc_priv *asrc_priv = asrc->private;
> -
> -	regmap_read(asrc->regmap, REG_ASRCFG,
> -		    &asrc_priv->regcache_cfg);
> -
> -	regcache_cache_only(asrc->regmap, true);
> -	regcache_mark_dirty(asrc->regmap);
> -
> -	return 0;
> -}
> -
> -static int fsl_asrc_resume(struct device *dev)
> -{
> -	struct fsl_asrc *asrc = dev_get_drvdata(dev);
> -	struct fsl_asrc_priv *asrc_priv = asrc->private;
> -	u32 asrctr;
> -
> -	/* Stop all pairs provisionally */
> -	regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
> -	regmap_update_bits(asrc->regmap, REG_ASRCTR,
> -			   ASRCTR_ASRCEi_ALL_MASK, 0);
> -
> -	/* Restore all registers */
> -	regcache_cache_only(asrc->regmap, false);
> -	regcache_sync(asrc->regmap);
> -
> -	regmap_update_bits(asrc->regmap, REG_ASRCFG,
> -			   ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
> -			   ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
> -
> -	/* Restart enabled pairs */
> -	regmap_update_bits(asrc->regmap, REG_ASRCTR,
> -			   ASRCTR_ASRCEi_ALL_MASK, asrctr);
> -
> -	return 0;
> -}
> -#endif /* CONFIG_PM_SLEEP */
> -
>  static const struct dev_pm_ops fsl_asrc_pm = {
>  	SET_RUNTIME_PM_OPS(fsl_asrc_runtime_suspend, fsl_asrc_runtime_resume, NULL)
> -	SET_SYSTEM_SLEEP_PM_OPS(fsl_asrc_suspend, fsl_asrc_resume)
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				pm_runtime_force_resume)
>  };
>  
>  static const struct fsl_asrc_soc_data fsl_asrc_imx35_data = {
> -- 
> 2.21.0
> 

Re: [PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume

[Shengjiu Wang]

On Mon, May 25, 2020 at 1:12 PM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Fri, May 22, 2020 at 05:57:24PM +0800, Shengjiu Wang wrote:
> > With dedicated power domain for asrc, power can be disabled after
> > probe and pm runtime suspend, then the value of all registers need to
> > be restored in pm runtime resume. So we can merge suspend/resume function
> > to runtime_suspend/resume function and enable regcache only in end of
> > probe.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> >  sound/soc/fsl/fsl_asrc.c | 70 ++++++++++++++++------------------------
> >  1 file changed, 27 insertions(+), 43 deletions(-)
> >
> > diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> > index 432936039de4..3ebbe15ac378 100644
> > --- a/sound/soc/fsl/fsl_asrc.c
> > +++ b/sound/soc/fsl/fsl_asrc.c
> > @@ -1100,6 +1100,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
> >       platform_set_drvdata(pdev, asrc);
> >       pm_runtime_enable(&pdev->dev);
> >       spin_lock_init(&asrc->lock);
> > +     regcache_cache_only(asrc->regmap, true);
> >
> >       ret = devm_snd_soc_register_component(&pdev->dev, &fsl_asrc_component,
> >                                             &fsl_asrc_dai, 1);
> > @@ -1117,6 +1118,7 @@ static int fsl_asrc_runtime_resume(struct device *dev)
> >       struct fsl_asrc *asrc = dev_get_drvdata(dev);
> >       struct fsl_asrc_priv *asrc_priv = asrc->private;
> >       int i, ret;
> > +     u32 asrctr;
> >
> >       ret = clk_prepare_enable(asrc->mem_clk);
> >       if (ret)
> > @@ -1135,6 +1137,24 @@ static int fsl_asrc_runtime_resume(struct device *dev)
> >                       goto disable_asrck_clk;
> >       }
> >
> > +     /* Stop all pairs provisionally */
> > +     regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
> > +     regmap_update_bits(asrc->regmap, REG_ASRCTR,
> > +                        ASRCTR_ASRCEi_ALL_MASK, 0);
> > +
> > +     /* Restore all registers */
> > +     regcache_cache_only(asrc->regmap, false);
> > +     regcache_mark_dirty(asrc->regmap);
>
>
> I see you doing regcache_mark_dirty() in the resume() now,
> being different from previously doing in suspend()?
>
> Thanks
> Nic

Which is for probe -> runtime_resume case.
After probe, the power may be disabled, so move mark_dirtry
to runtime_resume, then regcache can re-write all the registers.

best regards
wang shengjiu

>
>
> > +     regcache_sync(asrc->regmap);
> > +
> > +     regmap_update_bits(asrc->regmap, REG_ASRCFG,
> > +                        ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
> > +                        ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
> > +
> > +     /* Restart enabled pairs */
> > +     regmap_update_bits(asrc->regmap, REG_ASRCTR,
> > +                        ASRCTR_ASRCEi_ALL_MASK, asrctr);
> > +
> >       return 0;
> >
> >  disable_asrck_clk:
> > @@ -1155,6 +1175,11 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
> >       struct fsl_asrc_priv *asrc_priv = asrc->private;
> >       int i;
> >
> > +     regmap_read(asrc->regmap, REG_ASRCFG,
> > +                 &asrc_priv->regcache_cfg);
> > +
> > +     regcache_cache_only(asrc->regmap, true);
> > +
> >       for (i = 0; i < ASRC_CLK_MAX_NUM; i++)
> >               clk_disable_unprepare(asrc_priv->asrck_clk[i]);
> >       if (!IS_ERR(asrc->spba_clk))
> > @@ -1166,51 +1191,10 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
> >  }
> >  #endif /* CONFIG_PM */
> >
> > -#ifdef CONFIG_PM_SLEEP
> > -static int fsl_asrc_suspend(struct device *dev)
> > -{
> > -     struct fsl_asrc *asrc = dev_get_drvdata(dev);
> > -     struct fsl_asrc_priv *asrc_priv = asrc->private;
> > -
> > -     regmap_read(asrc->regmap, REG_ASRCFG,
> > -                 &asrc_priv->regcache_cfg);
> > -
> > -     regcache_cache_only(asrc->regmap, true);
> > -     regcache_mark_dirty(asrc->regmap);
> > -
> > -     return 0;
> > -}
> > -
> > -static int fsl_asrc_resume(struct device *dev)
> > -{
> > -     struct fsl_asrc *asrc = dev_get_drvdata(dev);
> > -     struct fsl_asrc_priv *asrc_priv = asrc->private;
> > -     u32 asrctr;
> > -
> > -     /* Stop all pairs provisionally */
> > -     regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
> > -     regmap_update_bits(asrc->regmap, REG_ASRCTR,
> > -                        ASRCTR_ASRCEi_ALL_MASK, 0);
> > -
> > -     /* Restore all registers */
> > -     regcache_cache_only(asrc->regmap, false);
> > -     regcache_sync(asrc->regmap);
> > -
> > -     regmap_update_bits(asrc->regmap, REG_ASRCFG,
> > -                        ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
> > -                        ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
> > -
> > -     /* Restart enabled pairs */
> > -     regmap_update_bits(asrc->regmap, REG_ASRCTR,
> > -                        ASRCTR_ASRCEi_ALL_MASK, asrctr);
> > -
> > -     return 0;
> > -}
> > -#endif /* CONFIG_PM_SLEEP */
> > -
> >  static const struct dev_pm_ops fsl_asrc_pm = {
> >       SET_RUNTIME_PM_OPS(fsl_asrc_runtime_suspend, fsl_asrc_runtime_resume, NULL)
> > -     SET_SYSTEM_SLEEP_PM_OPS(fsl_asrc_suspend, fsl_asrc_resume)
> > +     SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> > +                             pm_runtime_force_resume)
> >  };
> >
> >  static const struct fsl_asrc_soc_data fsl_asrc_imx35_data = {
> > --
> > 2.21.0
> >

Re: [PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume

[Shengjiu Wang]

On Mon, May 25, 2020 at 1:12 PM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Fri, May 22, 2020 at 05:57:24PM +0800, Shengjiu Wang wrote:
> > With dedicated power domain for asrc, power can be disabled after
> > probe and pm runtime suspend, then the value of all registers need to
> > be restored in pm runtime resume. So we can merge suspend/resume function
> > to runtime_suspend/resume function and enable regcache only in end of
> > probe.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> >  sound/soc/fsl/fsl_asrc.c | 70 ++++++++++++++++------------------------
> >  1 file changed, 27 insertions(+), 43 deletions(-)
> >
> > diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> > index 432936039de4..3ebbe15ac378 100644
> > --- a/sound/soc/fsl/fsl_asrc.c
> > +++ b/sound/soc/fsl/fsl_asrc.c
> > @@ -1100,6 +1100,7 @@ static int fsl_asrc_probe(struct platform_device *pdev)
> >       platform_set_drvdata(pdev, asrc);
> >       pm_runtime_enable(&pdev->dev);
> >       spin_lock_init(&asrc->lock);
> > +     regcache_cache_only(asrc->regmap, true);
> >
> >       ret = devm_snd_soc_register_component(&pdev->dev, &fsl_asrc_component,
> >                                             &fsl_asrc_dai, 1);
> > @@ -1117,6 +1118,7 @@ static int fsl_asrc_runtime_resume(struct device *dev)
> >       struct fsl_asrc *asrc = dev_get_drvdata(dev);
> >       struct fsl_asrc_priv *asrc_priv = asrc->private;
> >       int i, ret;
> > +     u32 asrctr;
> >
> >       ret = clk_prepare_enable(asrc->mem_clk);
> >       if (ret)
> > @@ -1135,6 +1137,24 @@ static int fsl_asrc_runtime_resume(struct device *dev)
> >                       goto disable_asrck_clk;
> >       }
> >
> > +     /* Stop all pairs provisionally */
> > +     regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
> > +     regmap_update_bits(asrc->regmap, REG_ASRCTR,
> > +                        ASRCTR_ASRCEi_ALL_MASK, 0);
> > +
> > +     /* Restore all registers */
> > +     regcache_cache_only(asrc->regmap, false);
> > +     regcache_mark_dirty(asrc->regmap);
>
>
> I see you doing regcache_mark_dirty() in the resume() now,
> being different from previously doing in suspend()?
>
> Thanks
> Nic

Which is for probe -> runtime_resume case.
After probe, the power may be disabled, so move mark_dirtry
to runtime_resume, then regcache can re-write all the registers.

best regards
wang shengjiu

>
>
> > +     regcache_sync(asrc->regmap);
> > +
> > +     regmap_update_bits(asrc->regmap, REG_ASRCFG,
> > +                        ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
> > +                        ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
> > +
> > +     /* Restart enabled pairs */
> > +     regmap_update_bits(asrc->regmap, REG_ASRCTR,
> > +                        ASRCTR_ASRCEi_ALL_MASK, asrctr);
> > +
> >       return 0;
> >
> >  disable_asrck_clk:
> > @@ -1155,6 +1175,11 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
> >       struct fsl_asrc_priv *asrc_priv = asrc->private;
> >       int i;
> >
> > +     regmap_read(asrc->regmap, REG_ASRCFG,
> > +                 &asrc_priv->regcache_cfg);
> > +
> > +     regcache_cache_only(asrc->regmap, true);
> > +
> >       for (i = 0; i < ASRC_CLK_MAX_NUM; i++)
> >               clk_disable_unprepare(asrc_priv->asrck_clk[i]);
> >       if (!IS_ERR(asrc->spba_clk))
> > @@ -1166,51 +1191,10 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
> >  }
> >  #endif /* CONFIG_PM */
> >
> > -#ifdef CONFIG_PM_SLEEP
> > -static int fsl_asrc_suspend(struct device *dev)
> > -{
> > -     struct fsl_asrc *asrc = dev_get_drvdata(dev);
> > -     struct fsl_asrc_priv *asrc_priv = asrc->private;
> > -
> > -     regmap_read(asrc->regmap, REG_ASRCFG,
> > -                 &asrc_priv->regcache_cfg);
> > -
> > -     regcache_cache_only(asrc->regmap, true);
> > -     regcache_mark_dirty(asrc->regmap);
> > -
> > -     return 0;
> > -}
> > -
> > -static int fsl_asrc_resume(struct device *dev)
> > -{
> > -     struct fsl_asrc *asrc = dev_get_drvdata(dev);
> > -     struct fsl_asrc_priv *asrc_priv = asrc->private;
> > -     u32 asrctr;
> > -
> > -     /* Stop all pairs provisionally */
> > -     regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
> > -     regmap_update_bits(asrc->regmap, REG_ASRCTR,
> > -                        ASRCTR_ASRCEi_ALL_MASK, 0);
> > -
> > -     /* Restore all registers */
> > -     regcache_cache_only(asrc->regmap, false);
> > -     regcache_sync(asrc->regmap);
> > -
> > -     regmap_update_bits(asrc->regmap, REG_ASRCFG,
> > -                        ASRCFG_NDPRi_ALL_MASK | ASRCFG_POSTMODi_ALL_MASK |
> > -                        ASRCFG_PREMODi_ALL_MASK, asrc_priv->regcache_cfg);
> > -
> > -     /* Restart enabled pairs */
> > -     regmap_update_bits(asrc->regmap, REG_ASRCTR,
> > -                        ASRCTR_ASRCEi_ALL_MASK, asrctr);
> > -
> > -     return 0;
> > -}
> > -#endif /* CONFIG_PM_SLEEP */
> > -
> >  static const struct dev_pm_ops fsl_asrc_pm = {
> >       SET_RUNTIME_PM_OPS(fsl_asrc_runtime_suspend, fsl_asrc_runtime_resume, NULL)
> > -     SET_SYSTEM_SLEEP_PM_OPS(fsl_asrc_suspend, fsl_asrc_resume)
> > +     SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> > +                             pm_runtime_force_resume)
> >  };
> >
> >  static const struct fsl_asrc_soc_data fsl_asrc_imx35_data = {
> > --
> > 2.21.0
> >

Re: [PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume

[Nicolin Chen]

On Mon, May 25, 2020 at 02:11:18PM +0800, Shengjiu Wang wrote:
> > > @@ -1135,6 +1137,24 @@ static int fsl_asrc_runtime_resume(struct device *dev)
> > >                       goto disable_asrck_clk;
> > >       }
> > >
> > > +     /* Stop all pairs provisionally */
> > > +     regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
> > > +     regmap_update_bits(asrc->regmap, REG_ASRCTR,
> > > +                        ASRCTR_ASRCEi_ALL_MASK, 0);
> > > +
> > > +     /* Restore all registers */
> > > +     regcache_cache_only(asrc->regmap, false);
> > > +     regcache_mark_dirty(asrc->regmap);
> >
> >
> > I see you doing regcache_mark_dirty() in the resume() now,
> > being different from previously doing in suspend()?

> Which is for probe -> runtime_resume case.
> After probe, the power may be disabled, so move mark_dirtry
> to runtime_resume, then regcache can re-write all the registers.

I see. Just noticed that you add a regcache_cache_only
in probe(). Acked already. Thanks.

Re: [PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume

[Nicolin Chen]

On Mon, May 25, 2020 at 02:11:18PM +0800, Shengjiu Wang wrote:
> > > @@ -1135,6 +1137,24 @@ static int fsl_asrc_runtime_resume(struct device *dev)
> > >                       goto disable_asrck_clk;
> > >       }
> > >
> > > +     /* Stop all pairs provisionally */
> > > +     regmap_read(asrc->regmap, REG_ASRCTR, &asrctr);
> > > +     regmap_update_bits(asrc->regmap, REG_ASRCTR,
> > > +                        ASRCTR_ASRCEi_ALL_MASK, 0);
> > > +
> > > +     /* Restore all registers */
> > > +     regcache_cache_only(asrc->regmap, false);
> > > +     regcache_mark_dirty(asrc->regmap);
> >
> >
> > I see you doing regcache_mark_dirty() in the resume() now,
> > being different from previously doing in suspend()?

> Which is for probe -> runtime_resume case.
> After probe, the power may be disabled, so move mark_dirtry
> to runtime_resume, then regcache can re-write all the registers.

I see. Just noticed that you add a regcache_cache_only
in probe(). Acked already. Thanks.

Re: [PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume

[Nicolin Chen]

On Fri, May 22, 2020 at 05:57:24PM +0800, Shengjiu Wang wrote:
> With dedicated power domain for asrc, power can be disabled after
> probe and pm runtime suspend, then the value of all registers need to
> be restored in pm runtime resume. So we can merge suspend/resume function
> to runtime_suspend/resume function and enable regcache only in end of
> probe.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>

Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>

Re: [PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume

[Nicolin Chen]

On Fri, May 22, 2020 at 05:57:24PM +0800, Shengjiu Wang wrote:
> With dedicated power domain for asrc, power can be disabled after
> probe and pm runtime suspend, then the value of all registers need to
> be restored in pm runtime resume. So we can merge suspend/resume function
> to runtime_suspend/resume function and enable regcache only in end of
> probe.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>

Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>

Re: [PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume

[Nicolin Chen]

On Fri, May 22, 2020 at 05:57:24PM +0800, Shengjiu Wang wrote:
> With dedicated power domain for asrc, power can be disabled after
> probe and pm runtime suspend, then the value of all registers need to
> be restored in pm runtime resume. So we can merge suspend/resume function
> to runtime_suspend/resume function and enable regcache only in end of
> probe.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>

Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>

Re: [PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume

[Mark Brown]

On Fri, 22 May 2020 17:57:24 +0800, Shengjiu Wang wrote:
> With dedicated power domain for asrc, power can be disabled after
> probe and pm runtime suspend, then the value of all registers need to
> be restored in pm runtime resume. So we can merge suspend/resume function
> to runtime_suspend/resume function and enable regcache only in end of
> probe.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume
      commit: 393dc21d0f25e8fcde8baca78b8a38afe61db2a7

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume

[Mark Brown]

On Fri, 22 May 2020 17:57:24 +0800, Shengjiu Wang wrote:
> With dedicated power domain for asrc, power can be disabled after
> probe and pm runtime suspend, then the value of all registers need to
> be restored in pm runtime resume. So we can merge suspend/resume function
> to runtime_suspend/resume function and enable regcache only in end of
> probe.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: fsl_asrc: Merge suspend/resume function to runtime_suspend/resume
      commit: 393dc21d0f25e8fcde8baca78b8a38afe61db2a7

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark