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

["Anson.Huang@freescale.com" <Anson.Huang@freescale.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 2347 bytes --]



Sent from Anson's iPhone

> ÔÚ 2013Äê12ÔÂ17ÈÕ£¬20:29£¬"Mark Brown" <broonie@kernel.org> дµÀ£º
> 
> On Tue, Dec 17, 2013 at 02:38:56AM +0000, Anson.Huang@freescale.com wrote:
> 
> Please fix the line length you're using to word wrap, it should be less
> than 80 columns to make your mails readable.
> 
>>>> +    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.
> 
> Why would that happen without the probe function failing - it would be
> better to add the error checking there wouldn't it?

Okay, than what about other functions? there is such condition check there is other functions too, that is why I add it here. if you think it is no necessary, I will remove this check in my patch?

> 
>> 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;
> 
> This sounds like you need to have some higher level synchronisation
> between whatever is managing this supply and your cpufreq driver - if
> you rely on reading back the current status from the hardware there will
> always be races between reading the state and the other thing doing the
> enable or disable.

yes, you are right. but I think we have handled that, all the operations of this LDO will via regulator interface, and regulator framework already has mutex lock. so there should be no such race in kernel as long as we all use regulator interface to access anatop LDO.ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥