linux-kernel - [RFC] Regulator state after regulator

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-next>] [day] [month] [year] [list]

Message-ID: <9D0D31AA57AAF5499AFDC63D6472631B09C76A@008-AM1MPN1-036.mgdnok.nokia.com>
Date:	Thu, 28 Apr 2011 09:01:03 +0000
From:	<kalle.jokiniemi@...ia.com>
To:	<broonie@...nsource.wolfsonmicro.com>, <lrg@...mlogic.co.uk>,
	<mchehab@...radead.org>, <svarbatov@...sol.com>,
	<saaguirre@...com>, <grosikopulos@...sol.com>,
	<vimarsh.zutshi@...ia.com>, <Sakari.Ailus@...ia.com>
CC:	<linux-kernel@...r.kernel.org>, <linux-media@...r.kernel.org>
Subject: [RFC] Regulator state after regulator_get 

Hello regulator FW and OMAP3 ISP fellows,

I'm currently optimizing power management for Nokia N900 MeeGo DE release,
and found an issue with how regulators are handled at boot.

The N900 uses VAUX2 regulator in OMAP3430 to power the CSIb IO complex
that is used by the camera. While implementing regulator FW support to
handle this regulator in the camera driver I noticed a problem with the
regulator init sequence:

If the device driver using the regulator does not enable and disable the
regulator after regulator_get, the regulator is left in the state that it was
after bootloader. In case of N900 this is a problem as the regulator is left
on to leak current. Of course there is the option to let regulator FW disable
all unused regulators, but this will break the N900 functionality, as the
regulator handling is not in place for many drivers. 

I found couple of solutions to this:
1. reset all regulators that have users (regulator_get is called on them) with
a regulator_enable/disable cycle within the regulator FW.
2. enable/disable the specific vdds_csib regulator in the omap3isp driver
to reset this one specific regulator to disabled state.

So, please share comments on which approach is more appropriate to take?
Or maybe there is option 3?

Here are example code for the two options (based on .37 kernel, will update
on top of appropriate tree once right solution is agreed):

Option1:

If a consumer device of a regulator gets a regulator, but
does not enable/disable it during probe, the regulator may
be left active from boot, even though it is not needed. If
it were needed it would be enabled by the consumer device.

So reset the regulator on first regulator_get call to make
sure that any regulator that has users is not left active
needlessly.

Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@...ia.com>
---
 drivers/regulator/core.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index ba521f0..040d850 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1152,6 +1152,12 @@ found:
 		module_put(rdev->owner);
 	}

+	/* Reset regulator to make sure it is disabled, if not needed */
+	if (!rdev->open_count) {
+		regulator_enable(regulator);
+		regulator_disable(regulator);
+	}
+
 	rdev->open_count++;
 	if (exclusive) {
 		rdev->exclusive = 1;
--

Option2:

Do a enable/disable cycle of the vdds_csib regulator to make
sure it is disabled after init in case there are no other
users for it. Otherwise regulator framework may leave it
active in case it was activated by bootloader.

Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@...ia.com>
---
 drivers/media/video/isp/ispccp2.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/isp/ispccp2.c b/drivers/media/video/isp/ispccp2.c
index 2d6b014..e11957f 100644
--- a/drivers/media/video/isp/ispccp2.c
+++ b/drivers/media/video/isp/ispccp2.c
@@ -1201,6 +1201,11 @@ int isp_ccp2_init(struct isp_device *isp)
 		}
 	}

+	if (ccp2->vdds_csib) {
+		regulator_enable(ccp2->vdds);
+		regulator_disable(ccp2->vdds);
+	}
+
 	ret = isp_ccp2_init_entities(ccp2);
 	if (ret < 0)
 		goto out;
--

Thanks,
Kalle

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/