Message-Id: <200901151432.31130.david-b@pacbell.net>
Date:	Thu, 15 Jan 2009 14:32:30 -0800
From:	David Brownell <david-b@...bell.net>
To:	Mark Brown <broonie@...ena.org.uk>
Cc:	Liam Girdwood <lrg@...mlogic.co.uk>,
	lkml <linux-kernel@...r.kernel.org>
Subject: Re: [patch 2.6.28-rc3] regulator: add REGULATOR_MODE_OFF

On Thursday 15 January 2009, Mark Brown wrote:
> > That raises an issue:  how can Linux get such regulators to
> > turn off?  Clock frameworks have the same issue, and they
> > tend to resolve this with a SoC-specific Kconfig option to
> > disable unused clocks (in a late_initcall, after everthing
> > has had a chance to start up).  That conserves power.
> 
> That's something that should be done by constraints - add a new flag
> that can be set.

That doesn't make any sense to me, any more than
adding a per-clock flag would.


> > I'm thinking it'd be worth having a similar Kconfig option
> > for regulators too.  It could kick in regulator core code to
> > call regulator_ops.disable() whenever regulator_dev.use_count
> > is zero, possibly warning if !regulator_ops.is_disabled().
> > (Handling constraints.always_on too.)
> 
> > Comments?
> 
> Given the model the API has of not doing anything unless explicitly
> told to

... e.g. by CONFIG_REGULATOR_DISABLE_UNUSED being explicitly
set, indicating that the regulator support for that platform
(and its drivers) is complete enough that it can safely paper
over such early init issues.


> I'd not expect to see this as a Kconfig option but rather as 
> something enabled per regulator or at least per board.

Why not, though?  That works well with clocks, once the
drivers are debugged.  Having dozens of fiddly little knobs
is not an advantage.

 
> The warning would be a bit interesting; ideally there should be one
> since it's probably an indication that something is wrong but there
> are probably going to be use cases for it that can't be covered by
> always_on, not that I can think of any right now.

The "boot_on" flag seems redundant with given "always_on"; as
a rule you can *tell* if the bootloader enabled a supply, so
the issue is more like whether it should stay on, even when no
driver wants it on.  Especially considering that bootloaders
seldom guarantee to enable only a minimal set of supplies.

See the appended patch.

- Dave


=========== CUT HERE
From: David Brownell <dbrownell@...rs.sourceforge.net>

Let the regulator framework disable regulators that have wrongly
been left enabled.  In the same manner as various implementations
of the clock framework, this is done by a late_initcall after all
drivers have had a chance to say what should be enabled.

Signed-off-by: David Brownell <dbrownell@...rs.sourceforge.net>
---
 drivers/regulator/Kconfig |   11 +++++++++++
 drivers/regulator/core.c  |   34 ++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -32,6 +32,17 @@ config REGULATOR_FIXED_VOLTAGE
 	tristate
 	default n
 
+config REGULATOR_DISABLE_UNUSED
+	bool "Disable unused regulators"
+	help
+	  Board initialization sometimes leaves a few regulators
+	  active even though they are not being used.  This option
+	  disables such regulators after all drivers have had a chance
+	  to initialize and enable their regulators, conserving power.
+
+	  If you're not sure that all your drivers have been properly
+	  integrated with the regulator framework, say no.
+
 config REGULATOR_VIRTUAL_CONSUMER
 	tristate "Virtual regulator consumer support"
 	default n
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -763,6 +763,7 @@ static int set_machine_constraints(struc
 			rdev->constraints = NULL;
 			goto out;
 		}
+		rdev->use_count = 1;
 	}
 
 	print_constraints(rdev);
@@ -2106,3 +2107,36 @@ static int __init regulator_init(void)
 
 /* init early to allow our consumers to complete system booting */
 core_initcall(regulator_init);
+
+static int __init regulator_disable_unused(void)
+{
+	struct regulator_dev *rdev;
+
+	mutex_lock(&regulator_list_mutex);
+	list_for_each_entry(rdev, &regulator_list, list) {
+		int	status;
+
+		if (rdev->use_count > 0)
+			continue;
+
+		status = _regulator_is_enabled(rdev);
+		if (status == 0)
+			continue;
+
+		if (status > 0)
+			dev_warn(&rdev->dev, "%s is unused but enabled\n",
+				rdev->desc->name);
+
+#ifdef CONFIG_REGULATOR_DISABLE_UNUSED
+		if (!rdev->desc->ops->disable)
+			continue;
+		status = rdev->desc->ops->disable(rdev);
+		if (status < 0)
+			dev_err(&rdev->dev, "%s error %d when disabling\n",
+					rdev->desc->name, status);
+#endif
+	}
+	mutex_unlock(&regulator_list_mutex);
+	return 0;
+}
+late_initcall(regulator_disable_unused);
--
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/

Powered by blists - more mailing lists