[v2,24/29] ASoC: tas2770: Add SDZ regulator

Commit Message

James Calligeros Feb. 18, 2025, 8:35 a.m. UTC

From: Hector Martin <[e-mail]>

Multiple amps can be connected to the same SDZ GPIO. Using raw GPIOs for
this breaks, as there is no concept of refcounting/sharing. In order to
model these platforms, introduce support for an SDZ "regulator". This
allows us to represent the SDZ GPIO as a simple regulator-fixed, and
then the regulator core takes care of refcounting so that all codecs are
only powered down once all the driver instances are in the suspend
state.

This also reworks the sleep/resume logic to copy what tas2764 does,
which makes more sense.

Reviewed-by: Neal Gompa <[e-mail]>
Signed-off-by: Hector Martin <[e-mail]>
Signed-off-by: James Calligeros <[e-mail]>
---
 sound/soc/codecs/tas2770.c | 67 ++++++++++++++++---------
 sound/soc/codecs/tas2770.h |  1 +
 2 files changed, 45 insertions(+), 23 deletions(-)

Comments

Mark Brown Feb. 18, 2025, 3:36 p.m. UTC | #1

On Tue, Feb 18, 2025 at 06:35:58PM +1000, James Calligeros wrote:
> From: Hector Martin <[e-mail]>
> 
> Multiple amps can be connected to the same SDZ GPIO. Using raw GPIOs for
> this breaks, as there is no concept of refcounting/sharing. In order to
> model these platforms, introduce support for an SDZ "regulator". This
> allows us to represent the SDZ GPIO as a simple regulator-fixed, and
> then the regulator core takes care of refcounting so that all codecs are
> only powered down once all the driver instances are in the suspend
> state.

Same issue with the use of the regulator API winding up as ABI here as
with the earlier patch - I get why you're doing this pragmatically and
we should just have a general helper for refcounted GPIO enables.

Patch

diff --git a/sound/soc/codecs/tas2770.c b/sound/soc/codecs/tas2770.c
index fee99db904a5885d740c1cfe8ce2645a963c6e1d..6f9e56f02f7ca95d7878c68465d7016213ae417a 100644
--- a/sound/soc/codecs/tas2770.c
+++ b/sound/soc/codecs/tas2770.c
@@ -72,23 +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;
 
-	regcache_cache_only(tas2770->regmap, true);
-	regcache_mark_dirty(tas2770->regmap);
+	ret = snd_soc_component_update_bits(component, TAS2770_PWR_CTRL,
+					    TAS2770_PWR_CTRL_MASK,
+					    TAS2770_PWR_CTRL_SHUTDOWN);
+	if (ret < 0)
+		return ret;
 
-	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;
-	}
+	regulator_disable(tas2770->sdz_reg);
+
+	regcache_cache_only(tas2770->regmap, true);
+	regcache_mark_dirty(tas2770->regmap);
+
+	usleep_range(6000, 7000);
 
 	return ret;
 }
@@ -98,18 +96,26 @@  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;
+	ret = regulator_enable(tas2770->sdz_reg);
+
+	if (ret) {
+		dev_err(tas2770->dev, "Failed to enable regulator\n");
+		return ret;
 	}
 
+	if (tas2770->sdz_gpio)
+		gpiod_set_value_cansleep(tas2770->sdz_gpio, 1);
+
+
+	usleep_range(1000, 2000);
+
 	regcache_cache_only(tas2770->regmap, false);
 
-	return regcache_sync(tas2770->regmap);
+	ret = regcache_sync(tas2770->regmap);
+	if (ret < 0)
+		return ret;
+
+	return tas2770_update_pwr_ctrl(tas2770);
 }
 #else
 #define tas2770_codec_suspend NULL
@@ -603,11 +609,18 @@  static int tas2770_codec_probe(struct snd_soc_component *component)
 
 	tas2770->component = component;
 
+	ret = regulator_enable(tas2770->sdz_reg);
+	if (ret != 0) {
+		dev_err(tas2770->dev, "Failed to enable regulator: %d\n", ret);
+		return ret;
+	}
+
 	if (tas2770->sdz_gpio) {
 		gpiod_set_value_cansleep(tas2770->sdz_gpio, 1);
-		usleep_range(1000, 2000);
 	}
 
+	usleep_range(1000, 2000);
+
 	tas2770_reset(tas2770);
 	regmap_reinit_cache(tas2770->regmap, &tas2770_i2c_regmap);
 
@@ -629,7 +642,10 @@  static int tas2770_codec_probe(struct snd_soc_component *component)
 
 static void tas2770_codec_remove(struct snd_soc_component *component)
 {
+	struct tas2770_priv *tas2770 = snd_soc_component_get_drvdata(component);
+
 	sysfs_remove_groups(&component->dev->kobj, tas2770_sysfs_groups);
+	regulator_disable(tas2770->sdz_reg);
 }
 
 static DECLARE_TLV_DB_SCALE(tas2770_digital_tlv, 1100, 50, 0);
@@ -769,6 +785,11 @@  static int tas2770_parse_dt(struct device *dev, struct tas2770_priv *tas2770)
 		tas2770->v_sense_slot = -1;
 	}
 
+	tas2770->sdz_reg = devm_regulator_get(dev, "SDZ");
+	if (IS_ERR(tas2770->sdz_reg))
+		return dev_err_probe(dev, PTR_ERR(tas2770->sdz_reg),
+				     "Failed to get SDZ supply\n");
+
 	tas2770->sdz_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH);
 	if (IS_ERR(tas2770->sdz_gpio)) {
 		if (PTR_ERR(tas2770->sdz_gpio) == -EPROBE_DEFER)
diff --git a/sound/soc/codecs/tas2770.h b/sound/soc/codecs/tas2770.h
index f75f40781ab136cccbe1c272f7129ddd3e4a22a3..f75baf23caf3a194a040474a7484a3d44f673435 100644
--- a/sound/soc/codecs/tas2770.h
+++ b/sound/soc/codecs/tas2770.h
@@ -134,6 +134,7 @@  struct tas2770_priv {
 	struct snd_soc_component *component;
 	struct gpio_desc *reset_gpio;
 	struct gpio_desc *sdz_gpio;
+	struct regulator *sdz_reg;
 	struct regmap *regmap;
 	struct device *dev;
 	int v_sense_slot;