RE: [PATCH] regulator: anatop: add is_enabled interface

[Anson.Huang@xxxxxxxxxxxxx]

Hi, Mark
	Thanks for your quick reply, see below inline:

Best Regards.
Anson huang éåæ
Â
Freescale Semiconductor Shanghai
äææäæåäæè192åAå2æ
201203
Tel:021-28937058


>-----Original Message-----
>From: Mark Brown [mailto:broonie@xxxxxxxxxx]
>Sent: Tuesday, December 17, 2013 2:58 AM
>To: Huang Yongcai-B20788
>Cc: lgirdwood@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
>Subject: Re: [PATCH] regulator: anatop: add is_enabled interface
>
>On Mon, Dec 16, 2013 at 04:20:27PM -0500, Anson Huang wrote:
>
>> +{
>> +	struct anatop_regulator *anatop_reg = rdev_get_drvdata(reg);
>> +	u32 val;
>> +
>> +	if (!anatop_reg->control_reg)
>> +		return -ENOTSUPP;
>
>In what situation would this happen and why would the operation be provided in
>those situations?
[Anson] This condition check is to avoid the case that control_reg is not
Initialized correctly in the probe function.
>
>> +	regmap_read(anatop_reg->anatop, anatop_reg->control_reg, &val);
>> +
>> +	return (val >> anatop_reg->vol_bit_shift) &
>> +		((1 << anatop_reg->vol_bit_width) - 1) ? 1 : 0; }
>
>This isn't terribly legible but it looks like it's trying to read to see if a
>non-zero voltage has been written.  If that's the case it looks like enable
>and disable operations should be supported since presumably the register can
>be written as well.

[Anson]Yes, from the register description, we can see that if the bit field of
REGx_TARG is not 0, then the LDO is enabled and there is output available. Adding this
Interface is because some specific requirement of our i.MX6's freq/volt scaling, and
We will add the enable/disable interface later when we add the GPU/VPU driver, as only
VDDPU can be on/off according to GPU/VPU's behavior, and there is a complicated flow
We need to follow, current is_enabled interface is for the case of VDDPU is turned off
In uboot.

1. For cpufreq change, we need to scale VDDARM as well as VDDSOC/PU's voltage, each
setpoint has different VDDARM, VDDSOC/PU voltage;
2. VDDARM and VDDSOC will be always on but VDDPU can be off in uboot due to the big leakage
it has(as high as ~43mA on i.MX6Q), as it is only for GPU/VPU module, and we will have
dynamic VDDPU power management available, so VDDPU LDO's status is changing according to GPU/VPU's
behavior;
3. If VDDPU is off during cpufreq change, then there is no need to scaling VDDPU's voltage,
so we will check whether VDDPU is enabled before scaling it in cpufreq driver, this is done
in my other patch thread;

>
>Please try to avoid the ternery operator, especially in cases like this where
>it's not adding anything.
[Anson] Accepted, will send out V2 patch if you think adding this interface for
our cpufreq driver is acceptable.
N‹§²æìr¸›yúèšØb²X¬¶ÇvØ^–)Þ{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü}©ž²ÆzÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹®w¥¢¸?™¨è­Ú&¢)ßf”ù^jÇy§m…á@A«a¶Úÿ0¶ìh®å’i