[PATCH 0/3] Add runtime PM for SAI digital audio interface

[PATCH 0/3] Add runtime PM for SAI digital audio interface

[Daniel Baluta]

First patch fixes a bug by correctly setting is_slave_mode, then
second patch adds support for runtime PM and finally 3rd patch moves
clock handling from startup/shtudown function to runtime PM handlers.

Changes since v1:
	- added patch 1
	- added fsl_sai_remove in order to call pm_runtime_disable
	- only disable/enable mclk when SAI in master mode.

Daniel Baluta (2):
  ASoC: fsl_sai: Update is_slave_mode with correct value
  ASoC: fsl_sai: Add support for runtime pm

Shengjiu Wang (1):
  ASoC: fsl_sai: Move clock operation to PM runtime

 sound/soc/fsl/fsl_sai.c | 84 +++++++++++++++++++++++++++++++++--------
 1 file changed, 69 insertions(+), 15 deletions(-)

-- 
2.17.1

[PATCH 1/3 v2] ASoC: fsl_sai: Update is_slave_mode with correct value

[Daniel Baluta]

is_slave_mode defaults to false because sai structure
that contains it is kzalloc'ed.

Anyhow, if we decide to set the following configuration
SAI slave -> SAI master, is_slave_mode will remain set on true
although SAI being master it should be set to false.

Fix this by updating is_slave_mode for each call of
fsl_sai_set_dai_fmt.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 sound/soc/fsl/fsl_sai.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index b563004fb89f..d9df98975cf8 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -268,12 +268,14 @@ static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai,
 	case SND_SOC_DAIFMT_CBS_CFS:
 		val_cr2 |= FSL_SAI_CR2_BCD_MSTR;
 		val_cr4 |= FSL_SAI_CR4_FSD_MSTR;
+		sai->is_slave_mode = false;
 		break;
 	case SND_SOC_DAIFMT_CBM_CFM:
 		sai->is_slave_mode = true;
 		break;
 	case SND_SOC_DAIFMT_CBS_CFM:
 		val_cr2 |= FSL_SAI_CR2_BCD_MSTR;
+		sai->is_slave_mode = false;
 		break;
 	case SND_SOC_DAIFMT_CBM_CFS:
 		val_cr4 |= FSL_SAI_CR4_FSD_MSTR;
-- 
2.17.1

Re: [PATCH 1/3 v2] ASoC: fsl_sai: Update is_slave_mode with correct value

[Nicolin Chen]

On Sun, Apr 21, 2019 at 07:39:08PM +0000, Daniel Baluta wrote:
> is_slave_mode defaults to false because sai structure
> that contains it is kzalloc'ed.
> 
> Anyhow, if we decide to set the following configuration
> SAI slave -> SAI master, is_slave_mode will remain set on true
> although SAI being master it should be set to false.
> 
> Fix this by updating is_slave_mode for each call of
> fsl_sai_set_dai_fmt.
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

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

Thanks

> ---
>  sound/soc/fsl/fsl_sai.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index b563004fb89f..d9df98975cf8 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -268,12 +268,14 @@ static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai,
>  	case SND_SOC_DAIFMT_CBS_CFS:
>  		val_cr2 |= FSL_SAI_CR2_BCD_MSTR;
>  		val_cr4 |= FSL_SAI_CR4_FSD_MSTR;
> +		sai->is_slave_mode = false;
>  		break;
>  	case SND_SOC_DAIFMT_CBM_CFM:
>  		sai->is_slave_mode = true;
>  		break;
>  	case SND_SOC_DAIFMT_CBS_CFM:
>  		val_cr2 |= FSL_SAI_CR2_BCD_MSTR;
> +		sai->is_slave_mode = false;
>  		break;
>  	case SND_SOC_DAIFMT_CBM_CFS:
>  		val_cr4 |= FSL_SAI_CR4_FSD_MSTR;
> -- 
> 2.17.1
> 

Applied "ASoC: fsl_sai: Update is_slave_mode with correct value" to the asoc tree

[Mark Brown]

The patch

   ASoC: fsl_sai: Update is_slave_mode with correct value

has been applied to the asoc tree at

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

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

From ddb351145a967ee791a0fb0156852ec2fcb746ba Mon Sep 17 00:00:00 2001
From: Daniel Baluta <daniel.baluta@nxp.com>
Date: Sun, 21 Apr 2019 19:39:08 +0000
Subject: [PATCH] ASoC: fsl_sai: Update is_slave_mode with correct value

is_slave_mode defaults to false because sai structure
that contains it is kzalloc'ed.

Anyhow, if we decide to set the following configuration
SAI slave -> SAI master, is_slave_mode will remain set on true
although SAI being master it should be set to false.

Fix this by updating is_slave_mode for each call of
fsl_sai_set_dai_fmt.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/fsl/fsl_sai.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index db9e0872f73d..7549b74e464e 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -268,12 +268,14 @@ static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai,
 	case SND_SOC_DAIFMT_CBS_CFS:
 		val_cr2 |= FSL_SAI_CR2_BCD_MSTR;
 		val_cr4 |= FSL_SAI_CR4_FSD_MSTR;
+		sai->is_slave_mode = false;
 		break;
 	case SND_SOC_DAIFMT_CBM_CFM:
 		sai->is_slave_mode = true;
 		break;
 	case SND_SOC_DAIFMT_CBS_CFM:
 		val_cr2 |= FSL_SAI_CR2_BCD_MSTR;
+		sai->is_slave_mode = false;
 		break;
 	case SND_SOC_DAIFMT_CBM_CFS:
 		val_cr4 |= FSL_SAI_CR4_FSD_MSTR;
-- 
2.20.1

Applied "ASoC: fsl_sai: Update is_slave_mode with correct value" to the asoc tree

[Mark Brown]

The patch

   ASoC: fsl_sai: Update is_slave_mode with correct value

has been applied to the asoc tree at

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

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

From ddb351145a967ee791a0fb0156852ec2fcb746ba Mon Sep 17 00:00:00 2001
From: Daniel Baluta <daniel.baluta@nxp.com>
Date: Sun, 21 Apr 2019 19:39:08 +0000
Subject: [PATCH] ASoC: fsl_sai: Update is_slave_mode with correct value

is_slave_mode defaults to false because sai structure
that contains it is kzalloc'ed.

Anyhow, if we decide to set the following configuration
SAI slave -> SAI master, is_slave_mode will remain set on true
although SAI being master it should be set to false.

Fix this by updating is_slave_mode for each call of
fsl_sai_set_dai_fmt.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/fsl/fsl_sai.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index db9e0872f73d..7549b74e464e 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -268,12 +268,14 @@ static int fsl_sai_set_dai_fmt_tr(struct snd_soc_dai *cpu_dai,
 	case SND_SOC_DAIFMT_CBS_CFS:
 		val_cr2 |= FSL_SAI_CR2_BCD_MSTR;
 		val_cr4 |= FSL_SAI_CR4_FSD_MSTR;
+		sai->is_slave_mode = false;
 		break;
 	case SND_SOC_DAIFMT_CBM_CFM:
 		sai->is_slave_mode = true;
 		break;
 	case SND_SOC_DAIFMT_CBS_CFM:
 		val_cr2 |= FSL_SAI_CR2_BCD_MSTR;
+		sai->is_slave_mode = false;
 		break;
 	case SND_SOC_DAIFMT_CBM_CFS:
 		val_cr4 |= FSL_SAI_CR4_FSD_MSTR;
-- 
2.20.1

[PATCH 2/3 v2] ASoC: fsl_sai: Add support for runtime pm

[Daniel Baluta]

Basically the same actions as for system PM, so make use
of pm_runtime_force_suspend/pm_runtime_force_resume.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 sound/soc/fsl/fsl_sai.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index d9df98975cf8..8623b7f882b9 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -9,6 +9,7 @@
 #include <linux/dmaengine.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/time.h>
@@ -900,6 +901,8 @@ static int fsl_sai_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, sai);
 
+	pm_runtime_enable(&pdev->dev);
+
 	ret = devm_snd_soc_register_component(&pdev->dev, &fsl_component,
 			&fsl_sai_dai, 1);
 	if (ret)
@@ -911,6 +914,11 @@ static int fsl_sai_probe(struct platform_device *pdev)
 		return devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
 }
 
+static int fsl_sai_remove(struct platform_device *pdev)
+{
+	pm_runtime_disable(&pdev->dev);
+}
+
 static const struct of_device_id fsl_sai_ids[] = {
 	{ .compatible = "fsl,vf610-sai", },
 	{ .compatible = "fsl,imx6sx-sai", },
@@ -919,8 +927,8 @@ static const struct of_device_id fsl_sai_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, fsl_sai_ids);
 
-#ifdef CONFIG_PM_SLEEP
-static int fsl_sai_suspend(struct device *dev)
+#ifdef CONFIG_PM
+static int fsl_sai_runtime_suspend(struct device *dev)
 {
 	struct fsl_sai *sai = dev_get_drvdata(dev);
 
@@ -930,7 +938,7 @@ static int fsl_sai_suspend(struct device *dev)
 	return 0;
 }
 
-static int fsl_sai_resume(struct device *dev)
+static int fsl_sai_runtime_resume(struct device *dev)
 {
 	struct fsl_sai *sai = dev_get_drvdata(dev);
 
@@ -942,14 +950,18 @@ static int fsl_sai_resume(struct device *dev)
 	regmap_write(sai->regmap, FSL_SAI_RCSR, 0);
 	return regcache_sync(sai->regmap);
 }
-#endif /* CONFIG_PM_SLEEP */
+#endif /* CONFIG_PM */
 
 static const struct dev_pm_ops fsl_sai_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(fsl_sai_suspend, fsl_sai_resume)
+	SET_RUNTIME_PM_OPS(fsl_sai_runtime_suspend,
+			   fsl_sai_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
 };
 
 static struct platform_driver fsl_sai_driver = {
 	.probe = fsl_sai_probe,
+	.remove = fsl_sai_remove,
 	.driver = {
 		.name = "fsl-sai",
 		.pm = &fsl_sai_pm_ops,
-- 
2.17.1

Re: [PATCH 2/3 v2] ASoC: fsl_sai: Add support for runtime pm

[Nicolin Chen]

On Sun, Apr 21, 2019 at 07:39:09PM +0000, Daniel Baluta wrote:
> Basically the same actions as for system PM, so make use
> of pm_runtime_force_suspend/pm_runtime_force_resume.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

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

Thanks

> ---
>  sound/soc/fsl/fsl_sai.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index d9df98975cf8..8623b7f882b9 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -9,6 +9,7 @@
>  #include <linux/dmaengine.h>
>  #include <linux/module.h>
>  #include <linux/of_address.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
>  #include <linux/time.h>
> @@ -900,6 +901,8 @@ static int fsl_sai_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, sai);
>  
> +	pm_runtime_enable(&pdev->dev);
> +
>  	ret = devm_snd_soc_register_component(&pdev->dev, &fsl_component,
>  			&fsl_sai_dai, 1);
>  	if (ret)
> @@ -911,6 +914,11 @@ static int fsl_sai_probe(struct platform_device *pdev)
>  		return devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
>  }
>  
> +static int fsl_sai_remove(struct platform_device *pdev)
> +{
> +	pm_runtime_disable(&pdev->dev);
> +}
> +
>  static const struct of_device_id fsl_sai_ids[] = {
>  	{ .compatible = "fsl,vf610-sai", },
>  	{ .compatible = "fsl,imx6sx-sai", },
> @@ -919,8 +927,8 @@ static const struct of_device_id fsl_sai_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(of, fsl_sai_ids);
>  
> -#ifdef CONFIG_PM_SLEEP
> -static int fsl_sai_suspend(struct device *dev)
> +#ifdef CONFIG_PM
> +static int fsl_sai_runtime_suspend(struct device *dev)
>  {
>  	struct fsl_sai *sai = dev_get_drvdata(dev);
>  
> @@ -930,7 +938,7 @@ static int fsl_sai_suspend(struct device *dev)
>  	return 0;
>  }
>  
> -static int fsl_sai_resume(struct device *dev)
> +static int fsl_sai_runtime_resume(struct device *dev)
>  {
>  	struct fsl_sai *sai = dev_get_drvdata(dev);
>  
> @@ -942,14 +950,18 @@ static int fsl_sai_resume(struct device *dev)
>  	regmap_write(sai->regmap, FSL_SAI_RCSR, 0);
>  	return regcache_sync(sai->regmap);
>  }
> -#endif /* CONFIG_PM_SLEEP */
> +#endif /* CONFIG_PM */
>  
>  static const struct dev_pm_ops fsl_sai_pm_ops = {
> -	SET_SYSTEM_SLEEP_PM_OPS(fsl_sai_suspend, fsl_sai_resume)
> +	SET_RUNTIME_PM_OPS(fsl_sai_runtime_suspend,
> +			   fsl_sai_runtime_resume, NULL)
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				pm_runtime_force_resume)
>  };
>  
>  static struct platform_driver fsl_sai_driver = {
>  	.probe = fsl_sai_probe,
> +	.remove = fsl_sai_remove,
>  	.driver = {
>  		.name = "fsl-sai",
>  		.pm = &fsl_sai_pm_ops,
> -- 
> 2.17.1
> 

Re: [PATCH 2/3 v2] ASoC: fsl_sai: Add support for runtime pm

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 244 bytes --]

On Sun, Apr 21, 2019 at 07:39:09PM +0000, Daniel Baluta wrote:
> Basically the same actions as for system PM, so make use
> of pm_runtime_force_suspend/pm_runtime_force_resume.

This doesn't apply against current code, please check and resend.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3 v2] ASoC: fsl_sai: Add support for runtime pm

[Daniel Baluta]

On Fri, Apr 26, 2019 at 1:01 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Sun, Apr 21, 2019 at 07:39:09PM +0000, Daniel Baluta wrote:
> > Basically the same actions as for system PM, so make use
> > of pm_runtime_force_suspend/pm_runtime_force_resume.
>
> This doesn't apply against current code, please check and resend.

Hi Mark,

This patch is already applied:

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?h=for-next&id=812ad463e0894d78e93ab8017e0edbe69038c55c

The only patch left in the series that needs to be applied is this:

https://www.spinics.net/lists/alsa-devel/msg89733.html

I will reply also to that email, to be easier for you to find it.

Re: [PATCH 2/3 v2] ASoC: fsl_sai: Add support for runtime pm

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 295 bytes --]

On Fri, Apr 26, 2019 at 03:10:10PM +0300, Daniel Baluta wrote:

> The only patch left in the series that needs to be applied is this:

> https://www.spinics.net/lists/alsa-devel/msg89733.html

> I will reply also to that email, to be easier for you to find it.

Content free pings don't help...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 3/3 v3] ASoC: fsl_sai: Move clock operation to PM runtime

[Daniel Baluta]

From: Shengjiu Wang <shengjiu.wang@nxp.com>

Turn off/on clocks when device enters suspend/resume. This
can help saving power.

As a further optimization, we turn off/on mclk only when SAI
is in master mode because otherwise mclk is externally provided.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 sound/soc/fsl/fsl_sai.c | 60 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 50 insertions(+), 10 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 8623b7f882b9..13a462360ed3 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -596,15 +596,8 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,
 {
 	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
 	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
-	struct device *dev = &sai->pdev->dev;
 	int ret;
 
-	ret = clk_prepare_enable(sai->bus_clk);
-	if (ret) {
-		dev_err(dev, "failed to enable bus clock: %d\n", ret);
-		return ret;
-	}
-
 	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx), FSL_SAI_CR3_TRCE,
 			   FSL_SAI_CR3_TRCE);
 
@@ -621,8 +614,6 @@ static void fsl_sai_shutdown(struct snd_pcm_substream *substream,
 	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
 
 	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx), FSL_SAI_CR3_TRCE, 0);
-
-	clk_disable_unprepare(sai->bus_clk);
 }
 
 static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
@@ -932,6 +923,16 @@ static int fsl_sai_runtime_suspend(struct device *dev)
 {
 	struct fsl_sai *sai = dev_get_drvdata(dev);
 
+	if (!sai->is_slave_mode) {
+		if (sai->mclk_streams & BIT(SNDRV_PCM_STREAM_CAPTURE))
+			clk_disable_unprepare(sai->mclk_clk[sai->mclk_id[0]]);
+
+		if (sai->mclk_streams & BIT(SNDRV_PCM_STREAM_PLAYBACK))
+			clk_disable_unprepare(sai->mclk_clk[sai->mclk_id[1]]);
+	}
+
+	clk_disable_unprepare(sai->bus_clk);
+
 	regcache_cache_only(sai->regmap, true);
 	regcache_mark_dirty(sai->regmap);
 
@@ -941,6 +942,27 @@ static int fsl_sai_runtime_suspend(struct device *dev)
 static int fsl_sai_runtime_resume(struct device *dev)
 {
 	struct fsl_sai *sai = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(sai->bus_clk);
+	if (ret) {
+		dev_err(dev, "failed to enable bus clock: %d\n", ret);
+		return ret;
+	}
+
+	if (!sai->is_slave_mode) {
+		if (sai->mclk_streams & BIT(SNDRV_PCM_STREAM_PLAYBACK)) {
+			ret = clk_prepare_enable(sai->mclk_clk[sai->mclk_id[1]]);
+			if (ret)
+				goto disable_bus_clk;
+		}
+
+		if (sai->mclk_streams & BIT(SNDRV_PCM_STREAM_CAPTURE)) {
+			ret = clk_prepare_enable(sai->mclk_clk[sai->mclk_id[0]]);
+			if (ret)
+				goto disable_tx_clk;
+		}
+	}
 
 	regcache_cache_only(sai->regmap, false);
 	regmap_write(sai->regmap, FSL_SAI_TCSR, FSL_SAI_CSR_SR);
@@ -948,7 +970,25 @@ static int fsl_sai_runtime_resume(struct device *dev)
 	usleep_range(1000, 2000);
 	regmap_write(sai->regmap, FSL_SAI_TCSR, 0);
 	regmap_write(sai->regmap, FSL_SAI_RCSR, 0);
-	return regcache_sync(sai->regmap);
+
+	ret = regcache_sync(sai->regmap);
+	if (ret)
+		goto disable_rx_clk;
+
+	return 0;
+
+disable_rx_clk:
+	if (!sai->is_slave_mode &&
+	    (sai->mclk_streams & BIT(SNDRV_PCM_STREAM_CAPTURE)))
+		clk_disable_unprepare(sai->mclk_clk[sai->mclk_id[0]]);
+disable_tx_clk:
+	if (!sai->is_slave_mode &&
+	    (sai->mclk_streams & BIT(SNDRV_PCM_STREAM_PLAYBACK)))
+		clk_disable_unprepare(sai->mclk_clk[sai->mclk_id[1]]);
+disable_bus_clk:
+	clk_disable_unprepare(sai->bus_clk);
+
+	return ret;
 }
 #endif /* CONFIG_PM */
 
-- 
2.17.1

Re: [PATCH 3/3 v3] ASoC: fsl_sai: Move clock operation to PM runtime

[Nicolin Chen]

On Sun, Apr 21, 2019 at 07:39:10PM +0000, Daniel Baluta wrote:
> From: Shengjiu Wang <shengjiu.wang@nxp.com>
> 
> Turn off/on clocks when device enters suspend/resume. This
> can help saving power.
> 
> As a further optimization, we turn off/on mclk only when SAI
> is in master mode because otherwise mclk is externally provided.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

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

Thanks

Re: [PATCH 3/3 v3] ASoC: fsl_sai: Move clock operation to PM runtime

[Viorel Suman]

Hi Daniel,

On Du, 2019-04-21 at 19:39 +0000, Daniel Baluta wrote:
> From: Shengjiu Wang <shengjiu.wang@nxp.com>
> 
> Turn off/on clocks when device enters suspend/resume. This
> can help saving power.
> 
> As a further optimization, we turn off/on mclk only when SAI
> is in master mode because otherwise mclk is externally provided.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
>  sound/soc/fsl/fsl_sai.c | 60 ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 50 insertions(+), 10 deletions(-)
> 
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index 8623b7f882b9..13a462360ed3 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -596,15 +596,8 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,
>  {
>  	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
>  	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
> -	struct device *dev = &sai->pdev->dev;
>  	int ret;
>  
> -	ret = clk_prepare_enable(sai->bus_clk);
> -	if (ret) {
> -		dev_err(dev, "failed to enable bus clock: %d\n", ret);
> -		return ret;
> -	}
> -
>  	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx), FSL_SAI_CR3_TRCE,
>  			   FSL_SAI_CR3_TRCE);
>  
> @@ -621,8 +614,6 @@ static void fsl_sai_shutdown(struct snd_pcm_substream *substream,
>  	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
>  
>  	regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx), FSL_SAI_CR3_TRCE, 0);
> -
> -	clk_disable_unprepare(sai->bus_clk);
>  }
>  
>  static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
> @@ -932,6 +923,16 @@ static int fsl_sai_runtime_suspend(struct device *dev)
>  {
>  	struct fsl_sai *sai = dev_get_drvdata(dev);
>  
> +	if (!sai->is_slave_mode) {
This check is redundant as the bits in sai->mclk_streams are set/unset for master
mode only, please check fsl_sai_hw_params and fsl_sai_hw_free functions.

> +		if (sai->mclk_streams & BIT(SNDRV_PCM_STREAM_CAPTURE))
> +			clk_disable_unprepare(sai->mclk_clk[sai->mclk_id[0]]);
> +
> +		if (sai->mclk_streams & BIT(SNDRV_PCM_STREAM_PLAYBACK))
> +			clk_disable_unprepare(sai->mclk_clk[sai->mclk_id[1]]);
> +	}
> +
> +	clk_disable_unprepare(sai->bus_clk);
> +
>  	regcache_cache_only(sai->regmap, true);
>  	regcache_mark_dirty(sai->regmap);
>  
> @@ -941,6 +942,27 @@ static int fsl_sai_runtime_suspend(struct device *dev)
>  static int fsl_sai_runtime_resume(struct device *dev)
>  {
>  	struct fsl_sai *sai = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = clk_prepare_enable(sai->bus_clk);
> +	if (ret) {
> +		dev_err(dev, "failed to enable bus clock: %d\n", ret);
> +		return ret;
> +	}
> +
> +	if (!sai->is_slave_mode) {
> +		if (sai->mclk_streams & BIT(SNDRV_PCM_STREAM_PLAYBACK)) {
> +			ret = clk_prepare_enable(sai->mclk_clk[sai->mclk_id[1]]);
> +			if (ret)
> +				goto disable_bus_clk;
> +		}
> +
> +		if (sai->mclk_streams & BIT(SNDRV_PCM_STREAM_CAPTURE)) {
> +			ret = clk_prepare_enable(sai->mclk_clk[sai->mclk_id[0]]);
> +			if (ret)
> +				goto disable_tx_clk;
> +		}
> +	}
>  
>  	regcache_cache_only(sai->regmap, false);
>  	regmap_write(sai->regmap, FSL_SAI_TCSR, FSL_SAI_CSR_SR);
> @@ -948,7 +970,25 @@ static int fsl_sai_runtime_resume(struct device *dev)
>  	usleep_range(1000, 2000);
>  	regmap_write(sai->regmap, FSL_SAI_TCSR, 0);
>  	regmap_write(sai->regmap, FSL_SAI_RCSR, 0);
> -	return regcache_sync(sai->regmap);
> +
> +	ret = regcache_sync(sai->regmap);
> +	if (ret)
> +		goto disable_rx_clk;
> +
> +	return 0;
> +
> +disable_rx_clk:
> +	if (!sai->is_slave_mode &&
> +	    (sai->mclk_streams & BIT(SNDRV_PCM_STREAM_CAPTURE)))
> +		clk_disable_unprepare(sai->mclk_clk[sai->mclk_id[0]]);
> +disable_tx_clk:
> +	if (!sai->is_slave_mode &&
> +	    (sai->mclk_streams & BIT(SNDRV_PCM_STREAM_PLAYBACK)))
> +		clk_disable_unprepare(sai->mclk_clk[sai->mclk_id[1]]);
> +disable_bus_clk:
> +	clk_disable_unprepare(sai->bus_clk);
> +
> +	return ret;
>  }
>  #endif /* CONFIG_PM */
>  
> -- 
> 2.17.1
> 

Re: [alsa-devel] [PATCH 3/3 v3] ASoC: fsl_sai: Move clock operation to PM runtime

[Daniel Baluta]

On Mon, Apr 22, 2019 at 2:07 PM Viorel Suman <viorel.suman@nxp.com> wrote:
>
> Hi Daniel,
>
> On Du, 2019-04-21 at 19:39 +0000, Daniel Baluta wrote:
> > From: Shengjiu Wang <shengjiu.wang@nxp.com>
> >
> > Turn off/on clocks when device enters suspend/resume. This
> > can help saving power.
> >
> > As a further optimization, we turn off/on mclk only when SAI
> > is in master mode because otherwise mclk is externally provided.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> > ---
> >  sound/soc/fsl/fsl_sai.c | 60 ++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 50 insertions(+), 10 deletions(-)
> >
> > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > index 8623b7f882b9..13a462360ed3 100644
> > --- a/sound/soc/fsl/fsl_sai.c
> > +++ b/sound/soc/fsl/fsl_sai.c
> > @@ -596,15 +596,8 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,
> >  {
> >       struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
> >       bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
> > -     struct device *dev = &sai->pdev->dev;
> >       int ret;
> >
> > -     ret = clk_prepare_enable(sai->bus_clk);
> > -     if (ret) {
> > -             dev_err(dev, "failed to enable bus clock: %d\n", ret);
> > -             return ret;
> > -     }
> > -
> >       regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx), FSL_SAI_CR3_TRCE,
> >                          FSL_SAI_CR3_TRCE);
> >
> > @@ -621,8 +614,6 @@ static void fsl_sai_shutdown(struct snd_pcm_substream *substream,
> >       bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
> >
> >       regmap_update_bits(sai->regmap, FSL_SAI_xCR3(tx), FSL_SAI_CR3_TRCE, 0);
> > -
> > -     clk_disable_unprepare(sai->bus_clk);
> >  }
> >
> >  static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
> > @@ -932,6 +923,16 @@ static int fsl_sai_runtime_suspend(struct device *dev)
> >  {
> >       struct fsl_sai *sai = dev_get_drvdata(dev);
> >
> > +     if (!sai->is_slave_mode) {
> This check is redundant as the bits in sai->mclk_streams are set/unset for master
> mode only, please check fsl_sai_hw_params and fsl_sai_hw_free functions.

Hi Viorel,

I think you make a very good point here. Will fix and send v3.

Thanks!