[PATCH 0/3] ASoC: mchp-spdiftx: add power saving features

[PATCH 0/3] ASoC: mchp-spdiftx: add power saving features

[Claudiu Beznea]

Hi,

This series adds support for runtime PM and system suspend/resume
for Microchip SPDIFTX (patches 2/3, 3/3). Along with it I took the
chance and added a minor cleanup (patch 1/3).

Thank you,
Claudiu Beznea

Claudiu Beznea (3):
  ASoC: mchp-spdiftx: simplify locking around ctrl->ch_stat
  ASoC: mchp-spdiftx: add runtime pm support
  ASoC: mchp-spdiftx: add support for system suspend/resume

 sound/soc/atmel/mchp-spdiftx.c | 165 ++++++++++++++++++++++-----------
 1 file changed, 113 insertions(+), 52 deletions(-)

-- 
2.34.1

[PATCH 1/3] ASoC: mchp-spdiftx: simplify locking around ctrl->ch_stat

[Claudiu Beznea]

Use a temporary variable to keep the AES3 value. With this a
spin_unlock_irqrestore() call has been removed from the final code.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdiftx.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdiftx.c b/sound/soc/atmel/mchp-spdiftx.c
index ab2d7a791f39..4e231cec9045 100644
--- a/sound/soc/atmel/mchp-spdiftx.c
+++ b/sound/soc/atmel/mchp-spdiftx.c
@@ -355,6 +355,7 @@ static int mchp_spdiftx_hw_params(struct snd_pcm_substream *substream,
 	struct mchp_spdiftx_mixer_control *ctrl = &dev->control;
 	u32 mr;
 	unsigned int bps = params_physical_width(params) / 8;
+	unsigned char aes3;
 	int ret;
 
 	dev_dbg(dev->dev, "%s() rate=%u format=%#x width=%u channels=%u\n",
@@ -440,48 +441,48 @@ static int mchp_spdiftx_hw_params(struct snd_pcm_substream *substream,
 
 	mr |= SPDIFTX_MR_BPS(bps);
 
-	spin_lock_irqsave(&ctrl->lock, flags);
-	ctrl->ch_stat[3] &= ~IEC958_AES3_CON_FS;
 	switch (params_rate(params)) {
 	case 22050:
-		ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_22050;
+		aes3 = IEC958_AES3_CON_FS_22050;
 		break;
 	case 24000:
-		ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_24000;
+		aes3 = IEC958_AES3_CON_FS_24000;
 		break;
 	case 32000:
-		ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_32000;
+		aes3 = IEC958_AES3_CON_FS_32000;
 		break;
 	case 44100:
-		ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_44100;
+		aes3 = IEC958_AES3_CON_FS_44100;
 		break;
 	case 48000:
-		ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_48000;
+		aes3 = IEC958_AES3_CON_FS_48000;
 		break;
 	case 88200:
-		ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_88200;
+		aes3 = IEC958_AES3_CON_FS_88200;
 		break;
 	case 96000:
-		ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_96000;
+		aes3 = IEC958_AES3_CON_FS_96000;
 		break;
 	case 176400:
-		ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_176400;
+		aes3 = IEC958_AES3_CON_FS_176400;
 		break;
 	case 192000:
-		ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_192000;
+		aes3 = IEC958_AES3_CON_FS_192000;
 		break;
 	case 8000:
 	case 11025:
 	case 16000:
 	case 64000:
-		ctrl->ch_stat[3] |= IEC958_AES3_CON_FS_NOTID;
+		aes3 = IEC958_AES3_CON_FS_NOTID;
 		break;
 	default:
 		dev_err(dev->dev, "unsupported sample frequency: %u\n",
 			params_rate(params));
-		spin_unlock_irqrestore(&ctrl->lock, flags);
 		return -EINVAL;
 	}
+	spin_lock_irqsave(&ctrl->lock, flags);
+	ctrl->ch_stat[3] &= ~IEC958_AES3_CON_FS;
+	ctrl->ch_stat[3] |= aes3;
 	mchp_spdiftx_channel_status_write(dev);
 	spin_unlock_irqrestore(&ctrl->lock, flags);
 
-- 
2.34.1

[PATCH 2/3] ASoC: mchp-spdiftx: add runtime pm support

[Claudiu Beznea]

Add runtime PM support for Microchip SPDIFTX driver. The runtime PM
APIs disables/enables IP's clock and enables/disable caching for
regmap.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdiftx.c | 116 ++++++++++++++++++++++++---------
 1 file changed, 86 insertions(+), 30 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdiftx.c b/sound/soc/atmel/mchp-spdiftx.c
index 4e231cec9045..ec454e64d85c 100644
--- a/sound/soc/atmel/mchp-spdiftx.c
+++ b/sound/soc/atmel/mchp-spdiftx.c
@@ -9,6 +9,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/pm_runtime.h>
 #include <linux/spinlock.h>
 
 #include <sound/asoundef.h>
@@ -175,6 +176,7 @@ static const struct regmap_config mchp_spdiftx_regmap_config = {
 	.readable_reg = mchp_spdiftx_readable_reg,
 	.writeable_reg = mchp_spdiftx_writeable_reg,
 	.precious_reg = mchp_spdiftx_precious_reg,
+	.cache_type = REGCACHE_FLAT,
 };
 
 #define SPDIFTX_GCLK_RATIO	128
@@ -196,7 +198,6 @@ struct mchp_spdiftx_dev {
 	struct clk				*pclk;
 	struct clk				*gclk;
 	unsigned int				fmt;
-	unsigned int				gclk_enabled:1;
 };
 
 static inline int mchp_spdiftx_is_running(struct mchp_spdiftx_dev *dev)
@@ -486,10 +487,9 @@ static int mchp_spdiftx_hw_params(struct snd_pcm_substream *substream,
 	mchp_spdiftx_channel_status_write(dev);
 	spin_unlock_irqrestore(&ctrl->lock, flags);
 
-	if (dev->gclk_enabled) {
-		clk_disable_unprepare(dev->gclk);
-		dev->gclk_enabled = 0;
-	}
+	/* GCLK is enabled by runtime PM. */
+	clk_disable_unprepare(dev->gclk);
+
 	ret = clk_set_rate(dev->gclk, params_rate(params) *
 				      SPDIFTX_GCLK_RATIO);
 	if (ret) {
@@ -503,7 +503,7 @@ static int mchp_spdiftx_hw_params(struct snd_pcm_substream *substream,
 		dev_err(dev->dev, "unable to enable gclk: %d\n", ret);
 		return ret;
 	}
-	dev->gclk_enabled = 1;
+
 	dev_dbg(dev->dev, "%s(): GCLK set to %d\n", __func__,
 		params_rate(params) * SPDIFTX_GCLK_RATIO);
 
@@ -523,10 +523,6 @@ static int mchp_spdiftx_hw_free(struct snd_pcm_substream *substream,
 
 	regmap_write(dev->regmap, SPDIFTX_IDR,
 		     SPDIFTX_IR_TXUDR | SPDIFTX_IR_TXOVR);
-	if (dev->gclk_enabled) {
-		clk_disable_unprepare(dev->gclk);
-		dev->gclk_enabled = 0;
-	}
 
 	return regmap_write(dev->regmap, SPDIFTX_CR,
 			    SPDIFTX_CR_SWRST | SPDIFTX_CR_FCLR);
@@ -709,17 +705,9 @@ static struct snd_kcontrol_new mchp_spdiftx_ctrls[] = {
 static int mchp_spdiftx_dai_probe(struct snd_soc_dai *dai)
 {
 	struct mchp_spdiftx_dev *dev = snd_soc_dai_get_drvdata(dai);
-	int ret;
 
 	snd_soc_dai_init_dma_data(dai, &dev->playback, NULL);
 
-	ret = clk_prepare_enable(dev->pclk);
-	if (ret) {
-		dev_err(dev->dev,
-			"failed to enable the peripheral clock: %d\n", ret);
-		return ret;
-	}
-
 	/* Add controls */
 	snd_soc_add_dai_controls(dai, mchp_spdiftx_ctrls,
 				 ARRAY_SIZE(mchp_spdiftx_ctrls));
@@ -727,19 +715,9 @@ static int mchp_spdiftx_dai_probe(struct snd_soc_dai *dai)
 	return 0;
 }
 
-static int mchp_spdiftx_dai_remove(struct snd_soc_dai *dai)
-{
-	struct mchp_spdiftx_dev *dev = snd_soc_dai_get_drvdata(dai);
-
-	clk_disable_unprepare(dev->pclk);
-
-	return 0;
-}
-
 static struct snd_soc_dai_driver mchp_spdiftx_dai = {
 	.name = "mchp-spdiftx",
 	.probe	= mchp_spdiftx_dai_probe,
-	.remove	= mchp_spdiftx_dai_remove,
 	.playback = {
 		.stream_name = "S/PDIF Playback",
 		.channels_min = 1,
@@ -763,6 +741,54 @@ static const struct of_device_id mchp_spdiftx_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, mchp_spdiftx_dt_ids);
 
+static int mchp_spdiftx_runtime_suspend(struct device *dev)
+{
+	struct mchp_spdiftx_dev *spdiftx = dev_get_drvdata(dev);
+
+	regcache_cache_only(spdiftx->regmap, true);
+
+	clk_disable_unprepare(spdiftx->gclk);
+	clk_disable_unprepare(spdiftx->pclk);
+
+	return 0;
+}
+
+static int mchp_spdiftx_runtime_resume(struct device *dev)
+{
+	struct mchp_spdiftx_dev *spdiftx = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(spdiftx->pclk);
+	if (ret) {
+		dev_err(spdiftx->dev,
+			"failed to enable the peripheral clock: %d\n", ret);
+		return ret;
+	}
+	ret = clk_prepare_enable(spdiftx->gclk);
+	if (ret) {
+		dev_err(spdiftx->dev,
+			"failed to enable generic clock: %d\n", ret);
+		goto disable_pclk;
+	}
+
+	regcache_cache_only(spdiftx->regmap, false);
+	regcache_mark_dirty(spdiftx->regmap);
+	ret = regcache_sync(spdiftx->regmap);
+	if (ret) {
+		regcache_cache_only(spdiftx->regmap, true);
+		clk_disable_unprepare(spdiftx->gclk);
+disable_pclk:
+		clk_disable_unprepare(spdiftx->pclk);
+	}
+
+	return ret;
+}
+
+static const struct dev_pm_ops mchp_spdiftx_pm_ops = {
+	RUNTIME_PM_OPS(mchp_spdiftx_runtime_suspend, mchp_spdiftx_runtime_resume,
+		       NULL)
+};
+
 static int mchp_spdiftx_probe(struct platform_device *pdev)
 {
 	struct mchp_spdiftx_dev *dev;
@@ -827,29 +853,59 @@ static int mchp_spdiftx_probe(struct platform_device *pdev)
 	dev->regmap = regmap;
 	platform_set_drvdata(pdev, dev);
 
+	pm_runtime_enable(dev->dev);
+	if (!pm_runtime_enabled(dev->dev)) {
+		err = mchp_spdiftx_runtime_resume(dev->dev);
+		if (err)
+			return err;
+	}
+
 	dev->playback.addr = (dma_addr_t)mem->start + SPDIFTX_CDR;
 	dev->playback.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 
 	err = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
 	if (err) {
 		dev_err(&pdev->dev, "failed to register PMC: %d\n", err);
-		return err;
+		goto pm_runtime_suspend;
 	}
 
 	err = devm_snd_soc_register_component(&pdev->dev,
 					      &mchp_spdiftx_component,
 					      &mchp_spdiftx_dai, 1);
-	if (err)
+	if (err) {
 		dev_err(&pdev->dev, "failed to register component: %d\n", err);
+		goto pm_runtime_suspend;
+	}
+
+	return 0;
+
+pm_runtime_suspend:
+	if (!pm_runtime_status_suspended(dev->dev))
+		mchp_spdiftx_runtime_suspend(dev->dev);
+	pm_runtime_disable(dev->dev);
 
 	return err;
 }
 
+static int mchp_spdiftx_remove(struct platform_device *pdev)
+{
+	struct mchp_spdiftx_dev *dev = platform_get_drvdata(pdev);
+
+	if (!pm_runtime_status_suspended(dev->dev))
+		mchp_spdiftx_runtime_suspend(dev->dev);
+
+	pm_runtime_disable(dev->dev);
+
+	return 0;
+}
+
 static struct platform_driver mchp_spdiftx_driver = {
 	.probe	= mchp_spdiftx_probe,
+	.remove = mchp_spdiftx_remove,
 	.driver	= {
 		.name	= "mchp_spdiftx",
 		.of_match_table = of_match_ptr(mchp_spdiftx_dt_ids),
+		.pm = pm_ptr(&mchp_spdiftx_pm_ops)
 	},
 };
 
-- 
2.34.1

[PATCH 3/3] ASoC: mchp-spdiftx: add support for system suspend/resume

[Claudiu Beznea]

Add support for system suspend/resume by moving the enable/disable
of interrupts in mchp_spdiftx_trigger() on SNDRV_PCM_TRIGGER_SUSPEND/
SNDRV_PCM_TRIGGER_RESUME commands.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 sound/soc/atmel/mchp-spdiftx.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/sound/soc/atmel/mchp-spdiftx.c b/sound/soc/atmel/mchp-spdiftx.c
index ec454e64d85c..dc96a6fbf514 100644
--- a/sound/soc/atmel/mchp-spdiftx.c
+++ b/sound/soc/atmel/mchp-spdiftx.c
@@ -198,6 +198,7 @@ struct mchp_spdiftx_dev {
 	struct clk				*pclk;
 	struct clk				*gclk;
 	unsigned int				fmt;
+	unsigned int				suspend_irq;
 };
 
 static inline int mchp_spdiftx_is_running(struct mchp_spdiftx_dev *dev)
@@ -318,16 +319,25 @@ static int mchp_spdiftx_trigger(struct snd_pcm_substream *substream, int cmd,
 	running = !!(mr & SPDIFTX_MR_TXEN_ENABLE);
 
 	switch (cmd) {
-	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_START:
+		regmap_write(dev->regmap, SPDIFTX_IER, dev->suspend_irq |
+			     SPDIFTX_IR_TXUDR | SPDIFTX_IR_TXOVR);
+		dev->suspend_irq = 0;
+		fallthrough;
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		if (!running) {
 			mr &= ~SPDIFTX_MR_TXEN_MASK;
 			mr |= SPDIFTX_MR_TXEN_ENABLE;
 		}
 		break;
-	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
+		regmap_read(dev->regmap, SPDIFTX_IMR, &dev->suspend_irq);
+		fallthrough;
+	case SNDRV_PCM_TRIGGER_STOP:
+		regmap_write(dev->regmap, SPDIFTX_IDR, dev->suspend_irq |
+			     SPDIFTX_IR_TXUDR | SPDIFTX_IR_TXOVR);
+		fallthrough;
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 		if (running) {
 			mr &= ~SPDIFTX_MR_TXEN_MASK;
@@ -507,10 +517,6 @@ static int mchp_spdiftx_hw_params(struct snd_pcm_substream *substream,
 	dev_dbg(dev->dev, "%s(): GCLK set to %d\n", __func__,
 		params_rate(params) * SPDIFTX_GCLK_RATIO);
 
-	/* Enable interrupts */
-	regmap_write(dev->regmap, SPDIFTX_IER,
-		     SPDIFTX_IR_TXUDR | SPDIFTX_IR_TXOVR);
-
 	regmap_write(dev->regmap, SPDIFTX_MR, mr);
 
 	return 0;
@@ -521,9 +527,6 @@ static int mchp_spdiftx_hw_free(struct snd_pcm_substream *substream,
 {
 	struct mchp_spdiftx_dev *dev = snd_soc_dai_get_drvdata(dai);
 
-	regmap_write(dev->regmap, SPDIFTX_IDR,
-		     SPDIFTX_IR_TXUDR | SPDIFTX_IR_TXOVR);
-
 	return regmap_write(dev->regmap, SPDIFTX_CR,
 			    SPDIFTX_CR_SWRST | SPDIFTX_CR_FCLR);
 }
@@ -785,6 +788,7 @@ static int mchp_spdiftx_runtime_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops mchp_spdiftx_pm_ops = {
+	SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
 	RUNTIME_PM_OPS(mchp_spdiftx_runtime_suspend, mchp_spdiftx_runtime_resume,
 		       NULL)
 };
-- 
2.34.1

Re: [PATCH 0/3] ASoC: mchp-spdiftx: add power saving features

[Mark Brown]

On Thu, 17 Nov 2022 14:37:47 +0200, Claudiu Beznea wrote:
> This series adds support for runtime PM and system suspend/resume
> for Microchip SPDIFTX (patches 2/3, 3/3). Along with it I took the
> chance and added a minor cleanup (patch 1/3).
> 
> Thank you,
> Claudiu Beznea
> 
> [...]

Applied to

   broonie/sound.git for-next

Thanks!

[1/3] ASoC: mchp-spdiftx: simplify locking around ctrl->ch_stat
      commit: 215450eb8b0fac000a42c1cd52c8966fb5159037
[2/3] ASoC: mchp-spdiftx: add runtime pm support
      commit: 4bf54ca60f99643cfaa3e5b532f139f6501f318e
[3/3] ASoC: mchp-spdiftx: add support for system suspend/resume
      commit: abc7edb0329cd2eabc0b948f5e248c85f6268296

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