Re: [RFC v2 07/13] power: pwrseq: simple: Add support for toggling regulator

[Krzysztof Kozlowski]

On 05/05/2016 09:31 PM, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote:
>> Some devices need real hard-reset by cutting the power.  During power
>> sequence turn off and on the regulator, if it is provided.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx>
>> ---
> 
> [snip]
> 
>>  
>>  #define to_pwrseq_simple(p) container_of(p, struct mmc_pwrseq_simple, pwrseq)
>> @@ -62,6 +65,13 @@ static void mmc_pwrseq_simple_post_power_on(struct pwrseq *_pwrseq)
>>  {
>>  	struct mmc_pwrseq_simple *pwrseq = to_pwrseq_simple(_pwrseq);
>>  
>> +	if (pwrseq->ext_reg) {
>> +		int err;
>> +
>> +		err = regulator_enable(pwrseq->ext_reg);
>> +		WARN_ON_ONCE(err);
>> +	}
>> +
> 
> Shouldn't this be in mmc_pwrseq_simple_pre_power_on() instead?
> 
> For example, a chip may need to be powered on before attempting to
> toggle its reset or power pins using some GPIO lines.

Indeed this should be still sorted out but here the assumption is that
regulator is disabled (by probe()) so it should be turned on with
GPIO-reset set.

This can be done at the end of pre-power-on or here (beginning of
post-power-on). On the other hand I understand these pre/post callbacks
as one starting the reset sequence (pre) and second as finishing it
(post). In case of regulators, finishing power sequence is to turn the
regulator on.

Best regards,
Krzysztof