[v2,14/19] ASoC: tlv320aic31xx: Remove regulator notification handling

Commit Message

Andrew Davis Nov. 29, 2017, 9:32 p.m. UTC

A regulator being forcefully disabled is a catastrophic event that
should never happen to most devices, especially not sound CODECs.
In addition, our handler sets the reset line but never disables it
as no one is listening for an enable event, this is certainly broken
and was mosy likely just copied from other CODECs, lets just remove
this code.

Signed-off-by: Andrew F. Davis <[e-mail]>
---
 sound/soc/codecs/tlv320aic31xx.c | 50 ----------------------------------------
 1 file changed, 50 deletions(-)

Comments

Mark Brown Dec. 1, 2017, 1:36 p.m. UTC | #1

On Wed, Nov 29, 2017 at 03:32:55PM -0600, Andrew F. Davis wrote:
> A regulator being forcefully disabled is a catastrophic event that
> should never happen to most devices, especially not sound CODECs.

That's not what the disable notification handling is for.  It's there so
that the driver can skip having to reinitialize the device if other
constraints mean the power doesn't actually get turned off when it
disables the regualtors.  It's nothing to do with forced disables.

Andrew Davis Dec. 1, 2017, 3:01 p.m. UTC | #2

On 12/01/2017 07:36 AM, Mark Brown wrote:
> On Wed, Nov 29, 2017 at 03:32:55PM -0600, Andrew F. Davis wrote:
>> A regulator being forcefully disabled is a catastrophic event that
>> should never happen to most devices, especially not sound CODECs.
> 
> That's not what the disable notification handling is for.  It's there so
> that the driver can skip having to reinitialize the device if other
> constraints mean the power doesn't actually get turned off when it
> disables the regualtors.  It's nothing to do with forced disables.
> 

Looking into the call sites, at least in this case the only time this
notification will be called, outside the normal enable/disable paths
(which do the same thing here: turn on regmap cache only mode and mark
it dirty), will be during a force disable scenario.

Mark Brown Dec. 1, 2017, 3:55 p.m. UTC | #3

On Fri, Dec 01, 2017 at 09:01:19AM -0600, Andrew F. Davis wrote:

> Looking into the call sites, at least in this case the only time this
> notification will be called, outside the normal enable/disable paths
> (which do the same thing here: turn on regmap cache only mode and mark
> it dirty), will be during a force disable scenario.

If all the disable sites in the driver mark the cache dirty then they
should be fixed not to.

Patch

diff --git a/sound/soc/codecs/tlv320aic31xx.c b/sound/soc/codecs/tlv320aic31xx.c
index f7b1ec96d826..b51c2777e8d1 100644
--- a/sound/soc/codecs/tlv320aic31xx.c
+++ b/sound/soc/codecs/tlv320aic31xx.c
@@ -161,7 +161,6 @@  struct aic31xx_priv {
 	struct gpio_desc *gpio_reset;
 	int micbias_vg;
 	struct regulator_bulk_data supplies[AIC31XX_NUM_SUPPLIES];
-	struct aic31xx_disable_nb disable_nb[AIC31XX_NUM_SUPPLIES];
 	unsigned int sysclk;
 	u8 p_div;
 	int rate_div_line;
@@ -1032,28 +1031,6 @@  static int aic31xx_set_dai_sysclk(struct snd_soc_dai *codec_dai,
 	return 0;
 }
 
-static int aic31xx_regulator_event(struct notifier_block *nb,
-				   unsigned long event, void *data)
-{
-	struct aic31xx_disable_nb *disable_nb =
-		container_of(nb, struct aic31xx_disable_nb, nb);
-	struct aic31xx_priv *aic31xx = disable_nb->aic31xx;
-
-	if (event & REGULATOR_EVENT_DISABLE) {
-		/*
-		 * Put codec to reset and as at least one of the
-		 * supplies was disabled.
-		 */
-		if (aic31xx->gpio_reset)
-			gpiod_set_value(aic31xx->gpio_reset, 1);
-
-		regcache_mark_dirty(aic31xx->regmap);
-		dev_dbg(aic31xx->dev, "## %s: DISABLE received\n", __func__);
-	}
-
-	return 0;
-}
-
 static void aic31xx_clk_on(struct snd_soc_codec *codec)
 {
 	struct aic31xx_priv *aic31xx = snd_soc_codec_get_drvdata(codec);
@@ -1167,20 +1144,6 @@  static int aic31xx_codec_probe(struct snd_soc_codec *codec)
 
 	aic31xx->codec = codec;
 
-	for (i = 0; i < ARRAY_SIZE(aic31xx->supplies); i++) {
-		aic31xx->disable_nb[i].nb.notifier_call =
-			aic31xx_regulator_event;
-		aic31xx->disable_nb[i].aic31xx = aic31xx;
-		ret = regulator_register_notifier(aic31xx->supplies[i].consumer,
-						  &aic31xx->disable_nb[i].nb);
-		if (ret) {
-			dev_err(codec->dev,
-				"Failed to request regulator notifier: %d\n",
-				ret);
-			return ret;
-		}
-	}
-
 	regcache_cache_only(aic31xx->regmap, true);
 	regcache_mark_dirty(aic31xx->regmap);
 
@@ -1195,21 +1158,8 @@  static int aic31xx_codec_probe(struct snd_soc_codec *codec)
 	return 0;
 }
 
-static int aic31xx_codec_remove(struct snd_soc_codec *codec)
-{
-	struct aic31xx_priv *aic31xx = snd_soc_codec_get_drvdata(codec);
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(aic31xx->supplies); i++)
-		regulator_unregister_notifier(aic31xx->supplies[i].consumer,
-					      &aic31xx->disable_nb[i].nb);
-
-	return 0;
-}
-
 static const struct snd_soc_codec_driver soc_codec_driver_aic31xx = {
 	.probe			= aic31xx_codec_probe,
-	.remove			= aic31xx_codec_remove,
 	.set_bias_level		= aic31xx_set_bias_level,
 	.suspend_bias_off	= true,