Re: [PATCH RFC 0/6] ASoC: sgtl5000: fix use of regulators and internal LDO

[Eric Nelson <eric.nelson@boundarydevices.com>]

Hi Mark,

On 02/26/2015 03:54 PM, Eric Nelson wrote:
> This patch set addresses a structural problem in the handling of regulators
> for VDDIO, VDDA, and VDDD in the SGTL5000 driver.
> 
> The first two of these power rails must be powered on prior to any I2C
> communication, and yet the regulators were tied to the codec, which is
> instantiated only after a fair amount of I2C communication takes place.
> 
> In other words, these regulators could never have function, and we can
> surmise that no user of this driver has switched power supply rails
> connected to them.
> 
> The third power rail (VDDD) can be derived internally (by using I2C registers)
> though the data sheet says that if an external VDDD is used, it should be
> enabled before MCLK is started and I2C activity begins.
> 
> Patch 1 moves the regulators to the I2C device and initializes them before
> 	I2C activity.
> Patch 2 addresses an issue found during development of the patch and is
>         somewhat unrelated to regulators. It is included for discussion
> Patch 3 removes the root cause of patch 2
> Patch 4 adds return value checks when setting the LINREG and ANA_POWER
> 	registers
> Patch 5 addresses the cause of follow-on failures and the source of the
> 	error in patch 3.
> Patch 6 removes the naive use of the supplies to lower power consumption
> 	and replaces it with something more modest.
> 
> The patch set is being sent as an RFC for discussion, and should probably
> be squashed into two or three patches covering the regulator updates
> (patches 1, 3-5 and perhaps 6), initialization error handling (patch 2).
> 
> Eric Nelson (6):
>   ASoC: sgtl5000: fix regulator support
>   ASoC: sgtl5000: write all default registers
>   ASoC: sgtl5000: initialize CHIP_ANA_POWER to power-on defaults
>   ASoC: sgtl5000: check return values
>   ASoC: sgtl5000: disable internal PLL early
>   ASoC: sgtl5000: Don't disable regulators in SND_SOC_BIAS_OFF
> 

Based on your review, it seems like the following is needed for a
V2 patch (set):

- Move call to sgtl5000_fill_defaults() earlier in the
  sgtl5000_i2c_probe routine, before adjusting ANA_POWER.
  This will allow registers LINEREG_CTRL, CLK_CTRL, and even ANA_POWER
  back into the default	register list.
- Move regulators from codec to I2C device
- switch to devm_regulator api
- various code cleanups to simplify logic
- Adjust regulator usage in SND_SOC_BIAS_OFF

I'll leave out the "write all default" registers for the moment.

Let me know if you'd like to see this in multiple patches or a
single patch.

I'll wait to see if there's other feedback before prepping V2.

Regards,


Eric