Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   James Calligeros <jcalligeros99@gmail.com>
To:     alsa-devel@alsa-project.org
Cc:     perex@perex.cz, tiwai@suse.com, povik+lin@cutebit.org,
        linux-kernel@vger.kernel.org,
        James Calligeros <jcalligeros99@gmail.com>
Subject: [PATCH] ASoC: tas27{64,70}: fix PM ops ordering
Date:   Fri,  2 Dec 2022 21:58:55 +1000
Message-Id: <20221202115855.16483-1-jcalligeros99@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On both tas2764 and tas2770, a write to PWR_CTRL is attempted
on resume before syncing the regcache to the chip, potentially leaving
it in an undefined state that causes resume to fail. The codec
is then unavailable until the next system reset.

On tas2770 specifically, both suspend and resume ops attempt useless
register writes on unrestored registers. This causes its state to be
desynchronised from what ASoC expects it to be in.

These two codecs are almost identical, so unify their behaviour
and reorder the ops so that the codec is always suspended and
resumed in a known/expected state.

Suggested-by: Martin Povišer <povik+lin@cutebit.org>
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 sound/soc/codecs/tas2764.c | 11 +++++++----
 sound/soc/codecs/tas2770.c | 40 ++++++++++++++++++++------------------
 2 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/sound/soc/codecs/tas2764.c b/sound/soc/codecs/tas2764.c
index 2e0ed3e68fa5..51c6b3a940c4 100644
--- a/sound/soc/codecs/tas2764.c
+++ b/sound/soc/codecs/tas2764.c
@@ -32,7 +32,7 @@ struct tas2764_priv {
 	struct regmap *regmap;
 	struct device *dev;
 	int irq;
-	
+
 	int v_sense_slot;
 	int i_sense_slot;
 
@@ -157,14 +157,17 @@ static int tas2764_codec_resume(struct snd_soc_component *component)
 		usleep_range(1000, 2000);
 	}
 
-	ret = tas2764_update_pwr_ctrl(tas2764);
+	regcache_cache_only(tas2764->regmap, false);
 
+	ret = regcache_sync(tas2764->regmap);
 	if (ret < 0)
 		return ret;
 
-	regcache_cache_only(tas2764->regmap, false);
+	ret = tas2764_update_pwr_ctrl(tas2764);
+	if (ret < 0)
+		return ret;
 
-	return regcache_sync(tas2764->regmap);
+	return 0;
 }
 #else
 #define tas2764_codec_suspend NULL
diff --git a/sound/soc/codecs/tas2770.c b/sound/soc/codecs/tas2770.c
index 8557759acb1f..5c9e8419b387 100644
--- a/sound/soc/codecs/tas2770.c
+++ b/sound/soc/codecs/tas2770.c
@@ -72,25 +72,21 @@ static int tas2770_codec_suspend(struct snd_soc_component *component)
 	struct tas2770_priv *tas2770 = snd_soc_component_get_drvdata(component);
 	int ret = 0;
 
+	ret = snd_soc_component_update_bits(component, TAS2770_PWR_CTRL,
+						TAS2770_PWR_CTRL_MASK,
+						TAS2770_PWR_CTRL_SHUTDOWN);
+
+	if (ret < 0)
+		return ret;
+
 	regcache_cache_only(tas2770->regmap, true);
-	regcache_mark_dirty(tas2770->regmap);
+	regcache_sync(tas2770->regmap);
 
-	if (tas2770->sdz_gpio) {
+	if (tas2770->sdz_gpio)
 		gpiod_set_value_cansleep(tas2770->sdz_gpio, 0);
-	} else {
-		ret = snd_soc_component_update_bits(component, TAS2770_PWR_CTRL,
-						    TAS2770_PWR_CTRL_MASK,
-						    TAS2770_PWR_CTRL_SHUTDOWN);
-		if (ret < 0) {
-			regcache_cache_only(tas2770->regmap, false);
-			regcache_sync(tas2770->regmap);
-			return ret;
-		}
 
-		ret = 0;
-	}
 
-	return ret;
+	return 0;
 }
 
 static int tas2770_codec_resume(struct snd_soc_component *component)
@@ -98,18 +94,24 @@ static int tas2770_codec_resume(struct snd_soc_component *component)
 	struct tas2770_priv *tas2770 = snd_soc_component_get_drvdata(component);
 	int ret;
 
+
 	if (tas2770->sdz_gpio) {
 		gpiod_set_value_cansleep(tas2770->sdz_gpio, 1);
 		usleep_range(1000, 2000);
-	} else {
-		ret = tas2770_update_pwr_ctrl(tas2770);
-		if (ret < 0)
-			return ret;
 	}
 
 	regcache_cache_only(tas2770->regmap, false);
 
-	return regcache_sync(tas2770->regmap);
+	ret = regcache_sync(tas2770->regmap);
+	if (ret < 0)
+		return ret;
+
+	ret = tas2770_update_pwr_ctrl(tas2770);
+	if (ret < 0)
+		return ret;
+
+
+	return 0;
 }
 #else
 #define tas2770_codec_suspend NULL
-- 
2.38.1