linux-kernel - RE: [PATCH] regulator: anatop: add is

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 17 Dec 2013 02:38:56 +0000
From:	"Anson.Huang@...escale.com" <Anson.Huang@...escale.com>
To:	Mark Brown <broonie@...nel.org>
CC:	"lgirdwood@...il.com" <lgirdwood@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] regulator: anatop: add is_enabled interface

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@...nel.org]
>Sent: Tuesday, December 17, 2013 2:58 AM
>To: Huang Yongcai-B20788
>Cc: lgirdwood@...il.com; linux-kernel@...r.kernel.org
>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.