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

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

Hi Russell,

On 03/06/2015 03:58 PM, Eric Nelson wrote:
> Hi Russell,
> 
> On 03/06/2015 03:53 PM, Russell King - ARM Linux wrote:
>> On Fri, Mar 06, 2015 at 03:49:39PM -0700, Eric Nelson wrote:
>>> 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.
>>
>> I'll also interject here to say that, against mainline, I need this
>> patch to make the sgtl5k work on SolidRun's Hummingboard hardware.
>> Without this, the internal regulator seems to get powered down, and
>> we lose access to the device until the entire board is power cycled.
>>
> 
> Mark applied essentially the patch below separately from
> the LDO support:
> 	http://mailman.alsa-project.org/pipermail/alsa-devel/2015-February/088376.html
> 	http://mailman.alsa-project.org/pipermail/alsa-devel/2015-March/088773.html
> 
>>  sound/soc/codecs/sgtl5000.c | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
>> index e182e6569bbd..79d5cbd65f36 100644
>> --- a/sound/soc/codecs/sgtl5000.c
>> +++ b/sound/soc/codecs/sgtl5000.c
>> @@ -825,6 +825,10 @@ static int ldo_regulator_disable(struct regulator_dev *dev)
>>  	struct snd_soc_codec *codec = (struct snd_soc_codec *)ldo->codec_data;
>>  

Sorry. This part of the patch you sent wasn't included in the
patch I mentioned above:

>>  	snd_soc_update_bits(codec, SGTL5000_CHIP_ANA_POWER,
>> +				SGTL5000_LINREG_SIMPLE_POWERUP,
>> +				SGTL5000_LINREG_SIMPLE_POWERUP);
>> +
>> +	snd_soc_update_bits(codec, SGTL5000_CHIP_ANA_POWER,
>>  				SGTL5000_LINEREG_D_POWERUP,
>>  				0);
>>  

In the full patch-set, initialization of ANA_POWER is placed inside
the sgtl5000_i2c_probe routine and the ldo_regulator is discarded.

I'm not sure why the code above is needed though.

Do you know what sequence of events causes this to fail?

Please advise,


Eric