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

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1563 bytes --]

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?

> 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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]