| Message ID | 1424991273-10081-7-git-send-email-eric.nelson@boundarydevices.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 26, 2015 at 03:54:33PM -0700, Eric Nelson wrote: > Disabling the SGTL5000 through regulators would certainly save > power than simply disabling the reference voltages as described > in the data sheet, but won't properly restore things on resume. Why not just fix the code, reinitializing the chip is usually pretty trivial?
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi Mark, On 03/06/2015 01:16 PM, Mark Brown wrote: > On Thu, Feb 26, 2015 at 03:54:33PM -0700, Eric Nelson wrote: >> Disabling the SGTL5000 through regulators would certainly save >> power than simply disabling the reference voltages as described >> in the data sheet, but won't properly restore things on resume. > > Why not just fix the code, reinitializing the chip is usually > pretty trivial? > Apparently not. I think it's fair to say that nobody has hooked up switching regulators using this driver, since they can't function without some form of this patch set. Once we have working regulator support, adding code to save power additional power here might make sense, but not until then. In other words, these are really elaborate no-ops now. Regards, Eric -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJU+iu4AAoJEFUqXmm9AiVrdlEH+gKLTIaT8ozcfbh1vgNWZQyz lTd6wlMK9InRxZU2UPXtcCUIyK+z0YnNNoZEaoIP9YYLadOmpC+Gw/lF+8bAXInt pEZqa17rLE7Q2xC1IhyjsIspQMP3Zpnvc/Ay51aq1I1M/wD4+G8kk6fhqrSAjBMo DqYCNmP5AbLNGL1yePp/Zt5G/mgApC3QtE4vdBdi/4HIFxMNwySEmVFQD0/WV2aI stZ8Sh5jc8pIQeLBRy1a0DJUF29jC7+nhVVMJQlXt8yWprnn6IgoM0dJTkzSif01 5HMRYLPbzemzJNOTcvIF15yOczGw4lqUmgbtR8Rz0wNQ3cghhjTk41HU5XasFJY= =/NEm -----END PGP SIGNATURE-----
On Fri, Mar 06, 2015 at 03:35:36PM -0700, Eric Nelson wrote: > On 03/06/2015 01:16 PM, Mark Brown wrote: > > Why not just fix the code, reinitializing the chip is usually > > pretty trivial? > Apparently not. Well, I'm not sure if that's actually true or not - the current driver code is pretty bad and there's been a history of worrying submissions that people assure me are working well and address problems. All of this is indicating to me that the complexity here is largely due to code quality issues and that if we address those then things might get a lot easier. Requiring a funny power on sequence shouldn't be hard and is needed for suspend and resume anyway, all having the regulator support does is mean we can potentially take advantage of that code path at runtime.
diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c index 5f7dd5d..d4d793f 100644 --- a/sound/soc/codecs/sgtl5000.c +++ b/sound/soc/codecs/sgtl5000.c @@ -767,36 +767,14 @@ static int sgtl5000_set_bias_level(struct snd_soc_codec *codec, switch (level) { case SND_SOC_BIAS_ON: case SND_SOC_BIAS_PREPARE: - break; case SND_SOC_BIAS_STANDBY: - if (codec->dapm.bias_level == SND_SOC_BIAS_OFF) { - ret = regulator_bulk_enable( - sgtl5000->num_supplies, - sgtl5000->supplies); - if (ret) - return ret; - udelay(10); - - regcache_cache_only(sgtl5000->regmap, false); - - ret = regcache_sync(sgtl5000->regmap); - if (ret != 0) { - dev_err(codec->dev, - "Failed to restore cache: %d\n", ret); - - regcache_cache_only(sgtl5000->regmap, true); - regulator_bulk_disable(sgtl5000->num_supplies, - sgtl5000->supplies); - - return ret; - } - } - + snd_soc_update_bits(codec, SGTL5000_CHIP_ANA_POWER, + SGTL5000_REFTOP_POWERUP, + SGTL5000_REFTOP_POWERUP); break; case SND_SOC_BIAS_OFF: - regcache_cache_only(sgtl5000->regmap, true); - regulator_bulk_disable(sgtl5000->num_supplies, - sgtl5000->supplies); + snd_soc_update_bits(codec, SGTL5000_CHIP_ANA_POWER, + SGTL5000_REFTOP_POWERUP, 0); break; }
Disabling the SGTL5000 through regulators would certainly save power than simply disabling the reference voltages as described in the data sheet, but won't properly restore things on resume. As patch 1 in the series shows, no user of this driver has support for active regulators yet, so this hasn't shown up. This patch is much more conservative and simply disables the reference bias currents. Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> --- sound/soc/codecs/sgtl5000.c | 32 +++++--------------------------- 1 file changed, 5 insertions(+), 27 deletions(-)