Updated V4 - Regulator Framework

This is an updated version of the kernel voltage & current regulator framework based on comments received from version 3 of the patch series.

The regulator framework is designed to provide a standard kernel interface to control voltage and current regulators on SoC based systems.

The intention is to allow systems to dynamically control regulator power output in order to save power and prolong battery life. This applies to both voltage regulators (where voltage output is controllable) and current sinks (where current limit is controllable).

Liam

oh gee.

I (and others) reviewed and commented on all this code last month. Now here it is again and, although I'm sure it is delightful code, nobody wants to have to read it all again ;)

The one-month gap between v3 and v4 is unfortunate. It means that everyone has forgotten everything.

Was all the review feedback satisfactorily handled to the reviewers' satisfaction?

Is there any new material in here which would require a re-review?

Sorry about this. It really couldn't be helped as I've been on the road a lot lately. I was even in the audience for your talk at CELF ;)

Summary of changes re V3 comments :-

o Added lots of documentation.
o Added ABI documentation.
o Changed debug option Kconfig style.
o Regulator no longer a class device.
o sysfs naming changes, ie. now uses microvolts and microamps.
o sysfs entries only export one value.
o get_current() macro undef removed.
o set_voltage(), set_current_limit() now take a lower and upper
acceptable value.
o Fixed a locking issue.
o Made 'compiled out' interface all static inline instead of macros.
o Removed dynamic voltage control where the voltage would be
dynamically managed to be the lowest for all consumers (no known users of this code atm - although spoke with someone at CELF who was interested in this feature for the future).
o Checkpatch fixes.
o Removed comments describing some functions from the headers - now all
such comments are in the c files.

Yes. Other additions :-

o Test harness added to aid developers with testing regulator drivers
(virtual.c - patch 07/13).
o Support for fixed output regulators. i.e. regulators that cannot be
controlled by software but coexist on systems with software controllable regulators (fixed.h, fixed.c - patches 04/13 and 06/13)
o Bulk operations for consumers that may have multiple regulator
supplies (consumer.h, core.c - patches 01/13 and 05/13)
o Changed core file and header names to be more meaningful. i.e
reg-core.c -> core.c, regulator/regulator.h -> regulator/consumer.h, etc.

Lastly, I've forgotten the MAINTAINERS patch but this can be added later.

Liam

Dear Liam,

I have reviewed your regulator framework from the point of view of the Openmoko GTA01 and GTA02 devices, based on members of the NXP PCF506xx PMU(PMIC) devices (for which I wrote the drivers).

I believe it would fit quite nice onto this PMIC family from a different manufacturer.

There is one aspects of the PCF506xx that I think the current regulator framework doesn't (yet) cover. I'm not sure if it was worth to add support for this, but let me explain:

The PCF506xx have a concept of global power management states, particularly important are the ON and STANDBY states in this context. Every regulator has a property whether it is enabled in none, one or both of the global power management states.

Do you think it would be worth to export something like this in the generic API, too? It's probably quite hard, since there are devices that actually use the PCF506xx STANDBY state during suspend-to-ram, but other devices leave the PCF506xx in the ON state and just disable the individual regulators using I2C register writes.

I think there's probably no clean way how to integrate this, since the question remains: who makes the decision (and manages the contraints) of
when to transition into the STANDBY state. Nevertheless a read-only
sysfs attribute might still be interesting.

Cheers,

- Harald Welte <[email protected]>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

That's a problem we've encountered - at least the WM8350 and (to a lesser extent) the WM8400, both of which have had support implemented in terms of the regulator API, have similar states.

Indeed - where a STANDBY-style state is used the likely implementation is going to be something like setting a GPIO at some point towards the very end of suspending to activate it (if it's done directly by software at all).

At present the regulator API pretty much works on the assumption that this will be very system specific and probably not amount to much in software terms since the CPU is likely to be suspended or off while these modes are active so the transitions can adequately be handled by machine specific code.

What might be more readily generalisable would be configuration for what happens in the very low power states. The WM8350 driver has a device-specific API which provides access for the soft configuration for this state since the chip allows the system to set the state of the regulator when it is suspended. This gives a second set of settings that are activated when the system enters a low power state. This caters for most cases (where this will be statically configured by the board setup code) but if it were generalised into the regulator core then it would allow use in conjunction with the device_*_wakeup() stuff, letting drivers say which of their supplies they need when suspended. I don't know how much demand there would be for that, though, and it'd be an extension to the API.

This gets rather more interesting when the system has a secondary PMIC - with multiple PMICs it's much more likely that one of them could be placed into a low power state while the system is running. Off the top of my head that may be adequately handled by having a master regulator driver in the tree supplying the secondary PMIC which puts it into a low power state when it is not needed. It's probably best tackled when we have some experience with such systems.

Fwiw, the WM8350 and the MC13783 also have similar global states for ON and STANDBY/HIBERNATE. I would imagine PMICs from other vendors would have similar global states too.

Yes I think it would be worthwhile adding this sort of functionality. I had considered this earlier on in development but it was lower priority for me then.

This would mean some additions to the regulator machine interface (to be used during machine regulator setup) and regulator driver interface (to set the values). I could add some 'standby_uV' and 'standby_mode' fields (one for each standby state) to struct regulation_constraints. This would define the desired voltage and regulator operating mode to be used during the regulators STANDBY/HIBERNATE states. Hence the correct regulator output state would be selected when the PMIC enters it's STANDBY/HIBERNATE state (via either a I2C write or hardware signal).

Would this suffice for your PMIC (sorry I don't have the datasheet) ?

Entering the PMICs global STANDBY/HIBERNATE state usually either comes from the hardware (e.g. button press, low battery, over temp) or a software user request. The constraints would be set at machine init time (along with the runtime constraints). This would work well for WM8350, but I'm not sure for PCF506xx.

However, entering individual regulator STANDBY/HIBERNATE states can be more difficult and as it is quite tightly coupled to the machine (and it's current use case or use case transition). Do you need per regulator STANDBY/HIBERNATE control ? We already have the ability to change the operating mode on a per regulator basis but not switch to states other than ON and OFF.

No problem, I'll add this too. I'll try and do this tomorrow (and also the MAINTAINERS patch) and send for review.

Cheers

Liam

ok. I also believe it is much lower priority. No disagreement here :)

basically the pcf506xx cannot support a different voltage in standby mode than in active mode. They just simply have something like a bitmaks where you define which regulator is still running in standby, and which don't.

You can change the bits via I2C, and you can enter STANDBY via either GPIO or I2C command.

As per the data sheet: If you really want to dig in to that, the PCF50633 is publicly available at
http://people.openmoko.org/tony_tu/GTA02/datasheet/PMU/PCF50633UM_6.pdf
and the PCF50606 is available at
http://www.rockbox.org/twiki/pub/Main/DataSheets/pcf50606.pdf

it's pretty much the same.

No!

Cheers,

- Harald Welte <[email protected]>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

Ok, I've created a new struct regulator_state in machine.h to describe a regulators suspended state for a given system suspend state :-

struct regulator_state {
int uV; /* suspend voltage */
unsigned int mode; /* suspend regulator operating mode */ int enabled; /* is regulator enabled in this suspend state */
};

'uV' and 'mode' are optional 'enabled' is mandatory. This should also cover other PMICs where voltage and operating mode may be configurable.

I've also added the following fields to struct regulation_constraints in machine.h :-

/* regulator suspend states for global PMIC STANDBY/HIBERNATE */ struct regulator_state state_disk;
struct regulator_state state_mem;
struct regulator_state state_standby;
suspend_state_t initial_state; /* suspend state to set at init */

This should define all the regulator's supported system suspend states and be registered with the other constraints.

In order to switch PMIC suspend configurations, I've added :-

int regulator_suspend_prepare(suspend_state_t state);

This will configure each PMIC regulator (if required) with the correct suspend setup prior to entering the new suspend state (and could be called by machine suspend code to prepare the pc506xx).

Liam

I've now finished the first draft of the system wide suspend state changes. This fits in well for WM8350 suspend configuration, could you give it a quick look over wrt pcf506xx family.

Changes :-

o regulator_suspend_prepare() added to set all the PMIC regulator
states before the system enters suspend. This would be called by your platform/machine specific suspend code.
o added sysfs entries for mem, disk and standby regulator config.
o added fields for each suspend state in struct regulation_constraints
to define each regulator suspend state.
o added operations to struct regulator_ops to support regulator
suspend state configuration in regulator drivers.

index f698f84..db259e8 100644
@@ -17,11 +17,12 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/mutex.h>
+#include <linux/suspend.h>
 #include <linux/regulator/consumer.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
 
-#define REGULATOR_VERSION "0.4"
+#define REGULATOR_VERSION "0.5"
 
 static DEFINE_MUTEX(regulator_list_mutex);
 static LIST_HEAD(regulator_list);

@@ -358,6 +359,126 @@ static ssize_t regulator_type_show(struct device *dev,
 	return sprintf(buf, "unknown\n");
 }
 
+static ssize_t regulator_suspend_mem_uV_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct regulator_dev *rdev = to_rdev(dev);
+
+	if (!rdev->constraints)
+		return sprintf(buf, "not defined\n");
+	return sprintf(buf, "%d\n", rdev->constraints->state_mem.uV);
+}
+
+static ssize_t regulator_suspend_disk_uV_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct regulator_dev *rdev = to_rdev(dev);
+
+	if (!rdev->constraints)
+		return sprintf(buf, "not defined\n");
+	return sprintf(buf, "%d\n", rdev->constraints->state_disk.uV);
+}
+
+static ssize_t regulator_suspend_standby_uV_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct regulator_dev *rdev = to_rdev(dev);
+
+	if (!rdev->constraints)
+		return sprintf(buf, "not defined\n");
+	return sprintf(buf, "%d\n", rdev->constraints->state_standby.uV);
+}
+
+static ssize_t suspend_opmode_show(struct regulator_dev *rdev,
+	unsigned int mode, char *buf)
+{
+	switch (mode) {
+	case REGULATOR_MODE_FAST:
+		return sprintf(buf, "fast\n");
+	case REGULATOR_MODE_NORMAL:
+		return sprintf(buf, "normal\n");
+	case REGULATOR_MODE_IDLE:
+		return sprintf(buf, "idle\n");
+	case REGULATOR_MODE_STANDBY:
+		return sprintf(buf, "standby\n");
+	}
+	return sprintf(buf, "unknown\n");
+}
+
+static ssize_t regulator_suspend_mem_mode_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct regulator_dev *rdev = to_rdev(dev);
+
+	if (!rdev->constraints)
+		return sprintf(buf, "not defined\n");
+	return suspend_opmode_show(rdev,
+		rdev->constraints->state_mem.mode, buf);
+}
+
+static ssize_t regulator_suspend_disk_mode_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct regulator_dev *rdev = to_rdev(dev);
+
+	if (!rdev->constraints)
+		return sprintf(buf, "not defined\n");
+	return suspend_opmode_show(rdev,
+		rdev->constraints->state_disk.mode, buf);
+}
+
+static ssize_t regulator_suspend_standby_mode_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct regulator_dev *rdev = to_rdev(dev);
+
+	if (!rdev->constraints)
+		return sprintf(buf, "not defined\n");
+	return suspend_opmode_show(rdev,
+		rdev->constraints->state_standby.mode, buf);
+}
+
+static ssize_t regulator_suspend_mem_state_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct regulator_dev *rdev = to_rdev(dev);
+
+	if (!rdev->constraints)
+		return sprintf(buf, "not defined\n");
+
+	if (rdev->constraints->state_mem.enabled)
+		return sprintf(buf, "enabled\n");
+	else
+		return sprintf(buf, "disabled\n");
+}
+
+static ssize_t regulator_suspend_disk_state_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct regulator_dev *rdev = to_rdev(dev);
+
+	if (!rdev->constraints)
+		return sprintf(buf, "not defined\n");
+
+	if (rdev->constraints->state_disk.enabled)
+		return sprintf(buf, "enabled\n");
+	else
+		return sprintf(buf, "disabled\n");
+}
+
+static ssize_t regulator_suspend_standby_state_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct regulator_dev *rdev = to_rdev(dev);
+
+	if (!rdev->constraints)
+		return sprintf(buf, "not defined\n");
+
+	if (rdev->constraints->state_standby.enabled)
+		return sprintf(buf, "enabled\n");
+	else
+		return sprintf(buf, "disabled\n");
+}
 static struct device_attribute regulator_dev_attrs[] = {
 	__ATTR(microvolts, 0444, regulator_uV_show, NULL),
 	__ATTR(microamps, 0444, regulator_uA_show, NULL),

@@ -370,6 +491,24 @@ static struct device_attribute regulator_dev_attrs[] = {
 	__ATTR(requested_microamps, 0444, regulator_total_uA_show, NULL),
 	__ATTR(num_users, 0444, regulator_num_users_show, NULL),
 	__ATTR(type, 0444, regulator_type_show, NULL),
+	__ATTR(suspend_mem_microvolts, 0444,
+		regulator_suspend_mem_uV_show, NULL),
+	__ATTR(suspend_disk_microvolts, 0444,
+		regulator_suspend_disk_uV_show, NULL),
+	__ATTR(suspend_standby_microvolts, 0444,
+		regulator_suspend_standby_uV_show, NULL),
+	__ATTR(suspend_mem_mode, 0444,
+		regulator_suspend_mem_mode_show, NULL),
+	__ATTR(suspend_disk_mode, 0444,
+		regulator_suspend_disk_mode_show, NULL),
+	__ATTR(suspend_standby_mode, 0444,
+		regulator_suspend_standby_mode_show, NULL),
+	__ATTR(suspend_mem_state, 0444,
+		regulator_suspend_mem_state_show, NULL),
+	__ATTR(suspend_disk_state, 0444,
+		regulator_suspend_disk_state_show, NULL),
+	__ATTR(suspend_standby_state, 0444,
+		regulator_suspend_standby_state_show, NULL),
 	__ATTR_NULL,
 };
 

@@ -425,6 +564,65 @@ static void drms_uA_update(struct regulator_dev *rdev)
 		rdev->desc->ops->set_mode(rdev, mode);
 }
 
+static int suspend_set_state(struct regulator_dev *rdev,
+	struct regulator_state *rstate)
+{
+	int ret = 0;
+
+	/* enable & disable are mandatory for suspend control */
+	if (!rdev->desc->ops->set_suspend_enable ||
+		!rdev->desc->ops->set_suspend_disable)
+		return -EINVAL;
+
+	if (rstate->enabled)
+		ret = rdev->desc->ops->set_suspend_enable(rdev);
+	else
+		ret = rdev->desc->ops->set_suspend_disable(rdev);
+	if (ret < 0) {
+		printk(KERN_ERR "%s: failed to enabled/disable\n", __func__);
+		return ret;
+	}
+
+	if (rdev->desc->ops->set_suspend_voltage && rstate->uV > 0) {
+		ret = rdev->desc->ops->set_suspend_voltage(rdev, rstate->uV);
+		if (ret < 0) {
+			printk(KERN_ERR "%s: failed to set voltage\n",
+				__func__);
+			return ret;
+		}
+	}
+
+	if (rdev->desc->ops->set_suspend_mode && rstate->mode > 0) {
+		ret = rdev->desc->ops->set_suspend_mode(rdev, rstate->mode);
+		if (ret < 0) {
+			printk(KERN_ERR "%s: failed to set mode\n", __func__);
+			return ret;
+		}
+	}
+	return ret;
+}
+
+/* locks held by caller */
+static int suspend_prepare(struct regulator_dev *rdev, suspend_state_t state)
+{
+	if (!rdev->constraints)
+		return -EINVAL;
+
+	switch (state) {
+	case PM_SUSPEND_STANDBY:
+		return suspend_set_state(rdev,
+			&rdev->constraints->state_standby);
+	case PM_SUSPEND_MEM:
+		return suspend_set_state(rdev,
+			&rdev->constraints->state_mem);
+	case PM_SUSPEND_MAX:
+		return suspend_set_state(rdev,
+			&rdev->constraints->state_disk);
+	default:
+		return -EINVAL;
+	}
+}
+
 static void print_constraints(struct regulator_dev *rdev)
 {
 	struct regulation_constraints *constraints = rdev->constraints;

@@ -923,7 +1121,7 @@ int regulator_get_voltage(struct regulator *regulator)
 EXPORT_SYMBOL_GPL(regulator_get_voltage);
 
 /**
- * regulator_set_current_limit - set regulator output current for a current sink
+ * regulator_set_current_limit - set regulator output current limit
  * @regulator: regulator source
  * @min_uA: Minimuum supported current in uA
  * @max_uA: Maximum supported current in uA

@@ -1567,6 +1765,10 @@ found:
 	if (rdev->constraints->boot_on)
 		rdev->use_count = 1;
 
+	/* do we need to setup our suspend state */
+	if (constraints->initial_state)
+		ret = suspend_prepare(rdev, constraints->initial_state);
+
 	print_constraints(rdev);
 	mutex_unlock(&rdev->mutex);
 

@@ -1612,6 +1814,41 @@ int regulator_set_device_supply(const char *regulator, struct device *dev,
 EXPORT_SYMBOL_GPL(regulator_set_device_supply);
 
 /**
+ * regulator_suspend_prepare: prepare regulators for system wide suspend
+ * @state: system suspend state
+ *
+ * Configure each regulator with it's suspend operating parameters for state.
+ * This will usually be called by machine suspend code prior to supending.
+ */
+int regulator_suspend_prepare(suspend_state_t state)
+{
+	struct regulator_dev *rdev;
+	int ret = 0;
+
+	/* ON is handled by regulator active state */
+	if (state == PM_SUSPEND_ON)
+		return -EINVAL;
+
+	mutex_lock(&regulator_list_mutex);
+	list_for_each_entry(rdev, &regulator_list, list) {
+
+		mutex_lock(&rdev->mutex);
+		ret = suspend_prepare(rdev, state);
+		mutex_unlock(&rdev->mutex);
+
+		if (ret < 0) {
+			printk(KERN_ERR "%s: failed to prepare %s\n",
+				__func__, rdev->desc->name);
+			goto out;
+		}
+	}
+out:
+	mutex_unlock(&regulator_list_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(regulator_suspend_prepare);
+
+/**
  * rdev_get_drvdata - get rdev regulator driver data
  * @regulator: regulator
  *

index 1c02362..1d712c7 100644
@@ -49,6 +49,19 @@ struct regulator_ops {
 	/* get most efficient regulator operating mode for load */
 	unsigned int (*get_optimum_mode) (struct regulator_dev *, int input_uV,
 					  int output_uV, int load_uA);
+
+	/* the operations below are for configuration of regulator state when
+	 * it's parent PMIC enters a global STANBY/HIBERNATE state */
+
+	/* set regulator suspend voltage */
+	int (*set_suspend_voltage) (struct regulator_dev *, int uV);
+
+	/* enable/disable regulator in suspend state */
+	int (*set_suspend_enable) (struct regulator_dev *);
+	int (*set_suspend_disable) (struct regulator_dev *);
+
+	/* set regulator suspend operating mode (defined in regulator.h) */
+	int (*set_suspend_mode) (struct regulator_dev *, unsigned int mode);
 };
 
 /*

index 69b492c..d3479a8 100644
@@ -39,6 +39,18 @@ struct regulator;
 #define REGULATOR_CHANGE_STATUS		0x8
 #define REGULATOR_CHANGE_DRMS		0x10
 
+
+/**
+ * struct regulator_state - regulator state during low power syatem states
+ *
+ * This describes a regulators state during a system wide low power state.
+ */
+struct regulator_state {
+	int uV;	/* suspend voltage */
+	unsigned int mode; /* suspend regulator operating mode */
+	int enabled; /* is regulator enabled in this suspend state */
+};
+
 /**
  * struct regulation_constraints - regulator operating constraints.
  *

@@ -65,6 +77,12 @@ struct regulation_constraints {
 	/* regulator input voltage - only if supply is another regulator */
 	int input_uV;
 
+	/* regulator suspend states for global PMIC STANDBY/HIBERNATE */
+	struct regulator_state state_disk;
+	struct regulator_state state_mem;
+	struct regulator_state state_standby;
+	suspend_state_t initial_state; /* suspend state to set at init */
+
 	/* constriant flags */
 	u32 always_on:1;	/* regulator never off when system is on */
 	u32 boot_on:1;		/* bootloader/firmware enabled regulator */

@@ -81,4 +99,6 @@ int regulator_set_machine_constraints(const char *regulator,
 int regulator_set_device_supply(const char *regulator, struct device *dev,
 				const char *supply);
 
+int regulator_suspend_prepare(suspend_state_t state);
+
 #endif

git-regulator.patch has been in -mm for two months without incident. I'd
forgotten about it. This is bad - we should have put it into linux-next.

If you hope to get this into 2.6.27 then please send the git URL to Stephen and the linux-next list asap. Please also resend the patches to this list for re-review. Then, if it doesn't get shot down, send Linux a pull request during the 2.6.27 merge window. ie: about one week from today.

Thanks.

Would there be any chance of this going in for rc2 ? (2 pull requests sent recently prior to rc1) or would it be better now waiting for the 2.6.28 merge window ?

Thanks

Liam

Oh, I'd assumed that it had been merged already.

Linus was probably wondering "who is this Liam person and why is he sending me stuff". It should be OK for 2.6.27-rc1 unless Linus had some specific reason for skipping it? Please retry.