The ASoC CODEC driver masks the IRQs whilst entering and exiting
system suspend to avoid issues where the IRQ handler can run but PM
runtime is disabled. However, as the IRQs could also be used from
other parts of the driver, it would be better to move this handling to
the MFD level.

Remove the handling from the ASoC driver and move it to the MFD
driver. Whilst moving also ensure the IRQs are all masked at the device
level before powering down the device, as per hardware recommendations.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/mfd/cs42l43.c      | 62 +++++++++++++++++++++++++++++++++-----
 sound/soc/codecs/cs42l43.c | 42 --------------------------
 2 files changed, 54 insertions(+), 50 deletions(-)

diff --git a/drivers/mfd/cs42l43.c b/drivers/mfd/cs42l43.c
index 3b4efb294471..e5f17fc430e4 100644
--- a/drivers/mfd/cs42l43.c
+++ b/drivers/mfd/cs42l43.c
@@ -1109,16 +1109,39 @@ EXPORT_SYMBOL_NS_GPL(cs42l43_dev_remove, MFD_CS42L43);
 static int cs42l43_suspend(struct device *dev)
 {
 	struct cs42l43 *cs42l43 = dev_get_drvdata(dev);
+	static const struct reg_sequence mask_all[] = {
+		{ CS42L43_DECIM_MASK,			0xFFFFFFFF, },
+		{ CS42L43_EQ_MIX_MASK,			0xFFFFFFFF, },
+		{ CS42L43_ASP_MASK,			0xFFFFFFFF, },
+		{ CS42L43_PLL_MASK,			0xFFFFFFFF, },
+		{ CS42L43_SOFT_MASK,			0xFFFFFFFF, },
+		{ CS42L43_SWIRE_MASK,			0xFFFFFFFF, },
+		{ CS42L43_MSM_MASK,			0xFFFFFFFF, },
+		{ CS42L43_ACC_DET_MASK,			0xFFFFFFFF, },
+		{ CS42L43_I2C_TGT_MASK,			0xFFFFFFFF, },
+		{ CS42L43_SPI_MSTR_MASK,		0xFFFFFFFF, },
+		{ CS42L43_SW_TO_SPI_BRIDGE_MASK,	0xFFFFFFFF, },
+		{ CS42L43_OTP_MASK,			0xFFFFFFFF, },
+		{ CS42L43_CLASS_D_AMP_MASK,		0xFFFFFFFF, },
+		{ CS42L43_GPIO_INT_MASK,		0xFFFFFFFF, },
+		{ CS42L43_ASRC_MASK,			0xFFFFFFFF, },
+		{ CS42L43_HPOUT_MASK,			0xFFFFFFFF, },
+	};
 	int ret;
 
-	/*
-	 * Don't care about being resumed here, but the driver does want
-	 * force_resume to always trigger an actual resume, so that register
-	 * state for the MCU/GPIOs is returned as soon as possible after system
-	 * resume. force_resume will resume if the reference count is resumed on
-	 * suspend hence the get_noresume.
-	 */
-	pm_runtime_get_noresume(dev);
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret) {
+		dev_err(cs42l43->dev, "Failed to resume for suspend: %d\n", ret);
+		return ret;
+	}
+
+	/* The IRQs will be re-enabled on resume by the cache sync */
+	ret = regmap_multi_reg_write_bypassed(cs42l43->regmap,
+					      mask_all, ARRAY_SIZE(mask_all));
+	if (ret) {
+		dev_err(cs42l43->dev, "Failed to mask IRQs: %d\n", ret);
+		return ret;
+	}
 
 	ret = pm_runtime_force_suspend(dev);
 	if (ret) {
@@ -1133,6 +1156,26 @@ static int cs42l43_suspend(struct device *dev)
 	if (ret)
 		return ret;
 
+	disable_irq(cs42l43->irq);
+
+	return 0;
+}
+
+static int cs42l43_suspend_noirq(struct device *dev)
+{
+	struct cs42l43 *cs42l43 = dev_get_drvdata(dev);
+
+	enable_irq(cs42l43->irq);
+
+	return 0;
+}
+
+static int cs42l43_resume_noirq(struct device *dev)
+{
+	struct cs42l43 *cs42l43 = dev_get_drvdata(dev);
+
+	disable_irq(cs42l43->irq);
+
 	return 0;
 }
 
@@ -1145,6 +1188,8 @@ static int cs42l43_resume(struct device *dev)
 	if (ret)
 		return ret;
 
+	enable_irq(cs42l43->irq);
+
 	ret = pm_runtime_force_resume(dev);
 	if (ret) {
 		dev_err(cs42l43->dev, "Failed to force resume: %d\n", ret);
@@ -1212,6 +1257,7 @@ static int cs42l43_runtime_resume(struct device *dev)
 
 EXPORT_NS_GPL_DEV_PM_OPS(cs42l43_pm_ops, MFD_CS42L43) = {
 	SYSTEM_SLEEP_PM_OPS(cs42l43_suspend, cs42l43_resume)
+	NOIRQ_SYSTEM_SLEEP_PM_OPS(cs42l43_suspend_noirq, cs42l43_resume_noirq)
 	RUNTIME_PM_OPS(cs42l43_runtime_suspend, cs42l43_runtime_resume, NULL)
 };
 
diff --git a/sound/soc/codecs/cs42l43.c b/sound/soc/codecs/cs42l43.c
index d0098b4558b5..3ede0e3110f3 100644
--- a/sound/soc/codecs/cs42l43.c
+++ b/sound/soc/codecs/cs42l43.c
@@ -2402,49 +2402,7 @@ static int cs42l43_codec_runtime_resume(struct device *dev)
 	return 0;
 }
 
-static int cs42l43_codec_suspend(struct device *dev)
-{
-	struct cs42l43_codec *priv = dev_get_drvdata(dev);
-	struct cs42l43 *cs42l43 = priv->core;
-
-	disable_irq(cs42l43->irq);
-
-	return 0;
-}
-
-static int cs42l43_codec_suspend_noirq(struct device *dev)
-{
-	struct cs42l43_codec *priv = dev_get_drvdata(dev);
-	struct cs42l43 *cs42l43 = priv->core;
-
-	enable_irq(cs42l43->irq);
-
-	return 0;
-}
-
-static int cs42l43_codec_resume(struct device *dev)
-{
-	struct cs42l43_codec *priv = dev_get_drvdata(dev);
-	struct cs42l43 *cs42l43 = priv->core;
-
-	enable_irq(cs42l43->irq);
-
-	return 0;
-}
-
-static int cs42l43_codec_resume_noirq(struct device *dev)
-{
-	struct cs42l43_codec *priv = dev_get_drvdata(dev);
-	struct cs42l43 *cs42l43 = priv->core;
-
-	disable_irq(cs42l43->irq);
-
-	return 0;
-}
-
 static const struct dev_pm_ops cs42l43_codec_pm_ops = {
-	SYSTEM_SLEEP_PM_OPS(cs42l43_codec_suspend, cs42l43_codec_resume)
-	NOIRQ_SYSTEM_SLEEP_PM_OPS(cs42l43_codec_suspend_noirq, cs42l43_codec_resume_noirq)
 	RUNTIME_PM_OPS(NULL, cs42l43_codec_runtime_resume, NULL)
 };
 
-- 
2.39.5

On Mon, 14 Oct 2024 10:52:02 +0100, Charles Keepax wrote:
> The ASoC CODEC driver masks the IRQs whilst entering and exiting
> system suspend to avoid issues where the IRQ handler can run but PM
> runtime is disabled. However, as the IRQs could also be used from
> other parts of the driver, it would be better to move this handling to
> the MFD level.
> 
> Remove the handling from the ASoC driver and move it to the MFD
> driver. Whilst moving also ensure the IRQs are all masked at the device
> level before powering down the device, as per hardware recommendations.
> 
> [...]

Applied, thanks!

[1/1] mfd: cs42l43: Disable IRQs during suspend
      commit: 2bb0106db90f86c3c513c26bb42ef0798ae8164d

--
Lee Jones [李琼斯]

On Mon, Oct 14, 2024 at 10:52:02AM +0100, Charles Keepax wrote:
> The ASoC CODEC driver masks the IRQs whilst entering and exiting
> system suspend to avoid issues where the IRQ handler can run but PM
> runtime is disabled. However, as the IRQs could also be used from
> other parts of the driver, it would be better to move this handling to
> the MFD level.

Acked-by: Mark Brown <broonie@kernel.org.

On Mon, 14 Oct 2024, Mark Brown wrote:

> On Mon, Oct 14, 2024 at 10:52:02AM +0100, Charles Keepax wrote:
> > The ASoC CODEC driver masks the IRQs whilst entering and exiting
> > system suspend to avoid issues where the IRQ handler can run but PM
> > runtime is disabled. However, as the IRQs could also be used from
> > other parts of the driver, it would be better to move this handling to
> > the MFD level.
> 
> Acked-by: Mark Brown <broonie@kernel.org.

Do you want a PR?

-- 
Lee Jones [李琼斯]

On Tue, Oct 15, 2024 at 03:34:26PM +0100, Lee Jones wrote:
> On Mon, 14 Oct 2024, Mark Brown wrote:
> 
> > On Mon, Oct 14, 2024 at 10:52:02AM +0100, Charles Keepax wrote:
> > > The ASoC CODEC driver masks the IRQs whilst entering and exiting
> > > system suspend to avoid issues where the IRQ handler can run but PM
> > > runtime is disabled. However, as the IRQs could also be used from
> > > other parts of the driver, it would be better to move this handling to
> > > the MFD level.
> > 
> > Acked-by: Mark Brown <broonie@kernel.org.
> 
> Do you want a PR?
> 

There's no other changes going through the ASoC part of the
driver for now, so should be fine to just pull this one
through MFD.

Thanks,
Charles