[PATCH v2 0/7] Add max77802 regulator operating mode support

[PATCH v2 0/7] Add max77802 regulator operating mode support

[Javier Martinez Canillas]

Hello Mark,

This is the second version of the series that adds operating modes
support for the regulators in the max77802 PMIC. It address issues
pointed out in "regulator: max77802: Add .{get,set}_mode callbacks"
and also removes the patches you already picked on the topic branch.
The first version of the series was [0].

The series adds a "maxim,regulator-initial-mode" property to configure
at startup the operating mode for the regulators that support changing
its mode during normal operation and "maxim,regulator-{disk,mem}-mode"
properties for the regulators that only support changing its operating
mode when the system enters in a suspend state.

The regulators can be enabled or disabled during suspend by using the
standard "regulator-{on,off}-in-suspend" properties from Chanwoo Choi's
regulator suspend state series [1].

I tried to use as much as possible the infrastructure that is already
provided in the regulator framework by adding the needed handlers for
the set_suspend_* operations.

Also, the driver still had some assumptions and didn't clearly made a
distinction between the valid modes (normal and low power) and off which
is not an operating mode as you explained. So I reworked a bit to better
treat them separately as is expected by the regulator API.

This series depend on [1] and also v2 of patch:
"ARM: EXYNOS: Call regulator core suspend prepare and finish functions" [2].

NOTE: Patch #4 touches a header in include/linux/mfd so some coordination
with the mfd maintainer (added as cc) will be needed.

Javier Martinez Canillas (7):
  regulator: max77802: Add .{get,set}_mode callbacks
  regulator: max77802: Add set suspend mode for BUCKs and simplify code
  regulator: max77802: Don't treat OFF as an operating mode
  regulator: max77802: Add header for operating modes
  regulator: max77802: Document regulator opmode DT properties
  regulator: max77802: Parse regulator operating mode properties
  ARM: dts: Configure regulators for suspend on exynos Peach boards

 .../devicetree/bindings/regulator/max77802.txt     |  45 ++++++++
 arch/arm/boot/dts/exynos5420-peach-pit.dts         |  81 ++++++++++++++
 arch/arm/boot/dts/exynos5800-peach-pi.dts          |  81 ++++++++++++++
 drivers/regulator/max77802.c                       | 122 ++++++++++++++++-----
 include/dt-bindings/regulator/maxim,max77802.h     |  18 +++
 include/linux/mfd/max77686.h                       |   7 --
 6 files changed, 319 insertions(+), 35 deletions(-)
 create mode 100644 include/dt-bindings/regulator/maxim,max77802.h

Patch #1 adds a get and set mode function handlers for the regulators whose
operating mode can be changed at runtime.

Patch #2 adds support for changing the operating mode for all regulators
that support setting a different opmode during suspend.

Patch #3 is a cleanup to not call OFF an operating mode.

Patch #4 adds a header file with the valid operating modes so it can be used
by Device Tree source files.

Patch #5 extend the max77802 DT binding to include the properties used to
setup the regulators modes.

Patch #6 adds the support to parse these from the driver.

Patch #7 configures the modes for the max77802 regulators in the Device Tree
source file of the Peach Chromebooks.

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/10/8/331
[1]: https://lkml.org/lkml/2014/10/10/161
[2]: http://www.spinics.net/lists/arm-kernel/msg369923.html

[PATCH v2 1/7] regulator: max77802: Add .{get,set}_mode callbacks

[Javier Martinez Canillas]

Some max77802 LDOs (1, 3, 20 and 21) support to be configured in Low
Power Mode during system normal operation. Add function handlers for
the .get_mode and .set_mode operations to set the mode on these LDOs.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

Changes since v1:
 - Use a static inline function instead of a macro to map the modes.
   Suggested by Mark Brown.
 - Don't have multiple standard modes mapping onto a single hw mode.
   Suggested by Mark Brown.

 drivers/regulator/max77802.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
index 8425c0d..6eabb95 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -70,6 +70,12 @@ struct max77802_regulator_prv {
 	unsigned int opmode[MAX77802_REG_MAX];
 };
 
+static inline int max77802_map_mode(int mode)
+{
+	return mode == MAX77802_OPMODE_NORMAL ?
+		REGULATOR_MODE_NORMAL : REGULATOR_MODE_STANDBY;
+}
+
 static int max77802_get_opmode_shift(int id)
 {
 	if (id == MAX77802_BUCK1 || (id >= MAX77802_BUCK5 &&
@@ -105,6 +111,44 @@ static int max77802_set_suspend_disable(struct regulator_dev *rdev)
 }
 
 /*
+ * Some LDOs support Low Power Mode while the system is running.
+ *
+ * LDOs 1, 3, 20, 21.
+ */
+static int max77802_set_mode(struct regulator_dev *rdev, unsigned int mode)
+{
+	struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	unsigned int val;
+	int shift = max77802_get_opmode_shift(id);
+
+	switch (mode) {
+	case REGULATOR_MODE_STANDBY:
+		val = MAX77802_OPMODE_LP;	/* ON in Low Power Mode */
+		break;
+	case REGULATOR_MODE_NORMAL:
+		val = MAX77802_OPMODE_NORMAL;	/* ON in Normal Mode */
+		break;
+	default:
+		dev_warn(&rdev->dev, "%s: regulator mode: 0x%x not supported\n",
+			 rdev->desc->name, mode);
+		return -EINVAL;
+	}
+
+	max77802->opmode[id] = val;
+	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
+				  rdev->desc->enable_mask, val << shift);
+}
+
+static unsigned max77802_get_mode(struct regulator_dev *rdev)
+{
+	struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+
+	return max77802_map_mode(max77802->opmode[id]);
+}
+
+/*
  * Some LDOs supports LPM-ON/OFF/Normal-ON mode during suspend state
  * (Enable Control Logic1 by PWRREQ)
  *
@@ -268,6 +312,8 @@ static struct regulator_ops max77802_ldo_ops_logic2 = {
 	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
 	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
 	.set_voltage_time_sel	= regulator_set_voltage_time_sel,
+	.set_mode		= max77802_set_mode,
+	.get_mode		= max77802_get_mode,
 	.set_suspend_mode	= max77802_ldo_set_suspend_mode_logic2,
 };
 
-- 
2.1.0

[PATCH v2 2/7] regulator: max77802: Add set suspend mode for BUCKs and simplify code

[Javier Martinez Canillas]

The max77802 PMIC has a special enable pin (PWRREQ) that can be used
by the Application Processor (AP) to power down and up voltage rails.

The max77802 PMIC regulators have 3 different enable control logics.
Some regulators support to be configured on different operational mode
during normal operation while others only support to be put in a Low
Power Mode while the system has entered in sleep mode. Some regulators
don't even support that configuration. The logics are the following:

Enable Control Logic1 by PWRREQ (BUCK 2-4, LDO2, LDO4-19, LDO22, LDO35)
-------------------------------
0: Output OFF
1: Output ON/OFF (Controlled by PWRREQ)
     PWRREQ = HIGH (1): Output ON in Normal Mode
     PWRREQ = LOW (0): Output OFF
2: Output On with Low Power Mode (Controlled by PWRREQ)
     PWRREQ = HIGH (1) : Output ON in Normal Mode
     PWRREQ = LOW (0): Output ON in Low Power Mode
3: Output ON in Normal Mode

Enable Control Logic2 by PWRREQ (LDO1, LDO20, LDO21)
-------------------------------
0: Output ON/OFF by ENx
1: Output ON in Low Power Mode
2: Output ON in Low Power Mode (Controlled by PWRREQ)
   PWRREQ = HIGH (1): Output ON in Normal Mode
   PWRREQ = LOW (0): Output ON in Low Power Mode
3: Output ON in Normal Mode

Enable Control Logic3 by PWRREQ (LDO3)
-------------------------------
0 or 3: Output ON in Normal Mode
1: Output ON in Low Power Mode
2: Output ON in Low Power Mode (Controlled by PWRREQ)
   PWRREQ = HIGH (1): Output ON in Normal Mode
   PWRREQ = LOW (0): Output ON in Low Power Mode

The driver only implemented .set_suspend_mode for the LDOs regulators
but some BUCKs also support to be put in Low Power Mode on system wide
suspend so they should be supported as well. Two different functions
were used for the logic 1 and 2 but this is not necessary.

Only normal and Low Power Mode are valid operational modes, OFF is not
an mode but is a regulator state that is handled by .set_suspend_enable
ad .set_suspend_disable. So the same .set_suspend_mode function can be
used by all the regulators that support Output On with Low Power Mode
by PWRREQ, making much simpler the code to set the suspend mode.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

Changes since v1:
 - Don't map both IDLE and STANDBY modes to the hw Low Power Mode.
   Suggested by Mark Brown.

 drivers/regulator/max77802.c | 93 ++++++++++++++++++++++----------------------
 1 file changed, 46 insertions(+), 47 deletions(-)

diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
index 6eabb95..dae2c1a 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -50,6 +50,7 @@
 #define MAX77802_RAMP_RATE_SHIFT_4BIT	4
 
 #define MAX77802_OFF_PWRREQ		0x1
+#define MAX77802_LP_PWRREQ		0x2
 
 /* MAX77802 has two register formats: 2-bit and 4-bit */
 static const unsigned int ramp_table_77802_2bit[] = {
@@ -148,71 +149,68 @@ static unsigned max77802_get_mode(struct regulator_dev *rdev)
 	return max77802_map_mode(max77802->opmode[id]);
 }
 
-/*
- * Some LDOs supports LPM-ON/OFF/Normal-ON mode during suspend state
- * (Enable Control Logic1 by PWRREQ)
+/**
+ * max77802_set_suspend_mode - set regulator opmode when the system is suspended
+ * @rdev: regulator to change mode
+ * @mode: operating mode to be set
+ *
+ * Will set the operating mode for the regulators during system suspend.
+ * This function is valid for the three different enable control logics:
  *
- * LDOs 2, 4-19, 22-35.
+ * Enable Control Logic1 by PWRREQ (BUCK 2-4 and LDOs 2, 4-19, 22-35)
+ * Enable Control Logic2 by PWRREQ (LDOs 1, 20, 21)
+ * Enable Control Logic3 by PWRREQ (LDO 3)
  *
+ * If setting the regulator mode fails, the function only warns but does
+ * not return an error code to avoid the regulator core to stop setting
+ * the operating mode for the remaining regulators.
  */
-static int max77802_ldo_set_suspend_mode_logic1(struct regulator_dev *rdev,
-						unsigned int mode)
+static int max77802_set_suspend_mode(struct regulator_dev *rdev,
+				     unsigned int mode)
 {
 	struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
 	int id = rdev_get_id(rdev);
 	unsigned int val;
 	int shift = max77802_get_opmode_shift(id);
 
-	switch (mode) {
-	case REGULATOR_MODE_IDLE:			/* ON in LP Mode */
-		val = MAX77802_OPMODE_LP;
-		break;
-	case REGULATOR_MODE_NORMAL:			/* ON in Normal Mode */
-		val = MAX77802_OPMODE_NORMAL;
-		break;
-	case REGULATOR_MODE_STANDBY:			/* ON/OFF by PWRREQ */
-		val = MAX77802_OPMODE_STANDBY;
-		break;
-	default:
-		dev_warn(&rdev->dev, "%s: regulator mode: 0x%x not supported\n",
+	/*
+	 * If the regulator has been disabled for suspend
+	 * then is invalid to try setting a suspend mode.
+	 */
+	if (!max77802->opmode[id] == MAX77802_OFF_PWRREQ) {
+		dev_warn(&rdev->dev, "%s: is disabled, mode: 0x%x not set\n",
 			 rdev->desc->name, mode);
-		return -EINVAL;
+		return 0;
 	}
 
-	max77802->opmode[id] = val;
-	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
-				  rdev->desc->enable_mask, val << shift);
-}
-
-/*
- * Mode 1 (Output[ON/OFF] by PWRREQ) is not supported on some LDOs
- * (Enable Control Logic2 by PWRREQ)
- *
- * LDOs 1, 20, 21, and 3,
- *
- */
-static int max77802_ldo_set_suspend_mode_logic2(struct regulator_dev *rdev,
-						unsigned int mode)
-{
-	struct max77802_regulator_prv *max77802 = rdev_get_drvdata(rdev);
-	int id = rdev_get_id(rdev);
-	unsigned int val;
-	int shift = max77802_get_opmode_shift(id);
-
 	switch (mode) {
-	case REGULATOR_MODE_IDLE:			/* ON in LP Mode */
-		val = MAX77802_OPMODE_LP;
-		break;
-	case REGULATOR_MODE_NORMAL:			/* ON in Normal Mode */
-		val = MAX77802_OPMODE_NORMAL;
+	case REGULATOR_MODE_STANDBY:
+		/*
+		 * If the regulator opmode is normal then enable
+		 * ON in Low Power Mode by PWRREQ. If the mode is
+		 * already Low Power then no action is required.
+		 */
+		if (max77802->opmode[id] == MAX77802_OPMODE_NORMAL)
+			val = MAX77802_LP_PWRREQ;
+		else
+			return 0;
 		break;
+	case REGULATOR_MODE_NORMAL:
+		/*
+		 * If the regulator operating mode is Low Power then
+		 * normal is not a valid opmode in suspend. If the
+		 * mode is already normal then no action is required.
+		 */
+		if (max77802->opmode[id] == MAX77802_OPMODE_LP)
+			dev_warn(&rdev->dev, "%s: in Low Power: 0x%x invalid\n",
+				 rdev->desc->name, mode);
+		return 0;
 	default:
 		dev_warn(&rdev->dev, "%s: regulator mode: 0x%x not supported\n",
 			 rdev->desc->name, mode);
 		return -EINVAL;
 	}
 
-	max77802->opmode[id] = val;
 	return regmap_update_bits(rdev->regmap, rdev->desc->enable_reg,
 				  rdev->desc->enable_mask, val << shift);
 }
@@ -297,7 +295,7 @@ static struct regulator_ops max77802_ldo_ops_logic1 = {
 	.set_voltage_time_sel	= regulator_set_voltage_time_sel,
 	.set_suspend_enable	= max77802_enable,
 	.set_suspend_disable	= max77802_set_suspend_disable,
-	.set_suspend_mode	= max77802_ldo_set_suspend_mode_logic1,
+	.set_suspend_mode	= max77802_set_suspend_mode,
 };
 
 /*
@@ -314,7 +312,7 @@ static struct regulator_ops max77802_ldo_ops_logic2 = {
 	.set_voltage_time_sel	= regulator_set_voltage_time_sel,
 	.set_mode		= max77802_set_mode,
 	.get_mode		= max77802_get_mode,
-	.set_suspend_mode	= max77802_ldo_set_suspend_mode_logic2,
+	.set_suspend_mode	= max77802_set_suspend_mode,
 };
 
 /* BUCKS 1, 6 */
@@ -345,6 +343,7 @@ static struct regulator_ops max77802_buck_234_ops = {
 	.set_ramp_delay		= max77802_set_ramp_delay_2bit,
 	.set_suspend_enable	= max77802_enable,
 	.set_suspend_disable	= max77802_set_suspend_disable,
+	.set_suspend_mode	= max77802_set_suspend_mode,
 };
 
 /* BUCKs 5, 7-10 */
-- 
2.1.0

[PATCH v2 3/7] regulator: max77802: Don't treat OFF as an operating mode

[Javier Martinez Canillas]

The only operating modes that are supported by the regulators in the
max77802 PMIC are Output ON (normal) and Output On in Low Power Mode.
OFF was wrongly counted as an operating mode while is only a regulator
status. Make clear in the code that OFF is not an operating mode.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

Changes since v1: None

 drivers/regulator/max77802.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
index dae2c1a..3abf99d 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -49,6 +49,7 @@
 #define MAX77802_RAMP_RATE_MASK_4BIT	0xF0
 #define MAX77802_RAMP_RATE_SHIFT_4BIT	4
 
+#define MAX77802_STATUS_OFF		0x0
 #define MAX77802_OFF_PWRREQ		0x1
 #define MAX77802_LP_PWRREQ		0x2
 
@@ -615,7 +616,7 @@ static int max77802_pmic_probe(struct platform_device *pdev)
 		 * the hardware reports OFF as the regulator operating mode.
 		 * Default to operating mode NORMAL in that case.
 		 */
-		if (val == MAX77802_OPMODE_OFF)
+		if (val == MAX77802_STATUS_OFF)
 			max77802->opmode[id] = MAX77802_OPMODE_NORMAL;
 		else
 			max77802->opmode[id] = val;
-- 
2.1.0

[PATCH v2 4/7] regulator: max77802: Add header for operating modes

[Javier Martinez Canillas]

Add a header file for the max77802 constants that could be shared between
the regulator driver and Device Tree source files. Also, remove standby
and off opmodes since only normal and low power are valid operating modes.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

Changes since v1: None

 drivers/regulator/max77802.c                   |  1 +
 include/dt-bindings/regulator/maxim,max77802.h | 18 ++++++++++++++++++
 include/linux/mfd/max77686.h                   |  7 -------
 3 files changed, 19 insertions(+), 7 deletions(-)
 create mode 100644 include/dt-bindings/regulator/maxim,max77802.h

diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
index 3abf99d..5839c45 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -33,6 +33,7 @@
 #include <linux/regulator/of_regulator.h>
 #include <linux/mfd/max77686.h>
 #include <linux/mfd/max77686-private.h>
+#include <dt-bindings/regulator/maxim,max77802.h>
 
 /* Default ramp delay in case it is not manually set */
 #define MAX77802_RAMP_DELAY		100000		/* uV/us */
diff --git a/include/dt-bindings/regulator/maxim,max77802.h b/include/dt-bindings/regulator/maxim,max77802.h
new file mode 100644
index 0000000..cf28631
--- /dev/null
+++ b/include/dt-bindings/regulator/maxim,max77802.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2014 Google, Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Device Tree binding constants for the Maxim 77802 PMIC regulators
+ */
+
+#ifndef _DT_BINDINGS_REGULATOR_MAXIM_MAX77802_H
+#define _DT_BINDINGS_REGULATOR_MAXIM_MAX77802_H
+
+/* Regulator operating modes */
+#define MAX77802_OPMODE_LP	1
+#define MAX77802_OPMODE_NORMAL	3
+
+#endif /* _DT_BINDINGS_REGULATOR_MAXIM_MAX77802_H */
diff --git a/include/linux/mfd/max77686.h b/include/linux/mfd/max77686.h
index 7e6dc4b..553f7d0 100644
--- a/include/linux/mfd/max77686.h
+++ b/include/linux/mfd/max77686.h
@@ -131,13 +131,6 @@ enum max77686_opmode {
 	MAX77686_OPMODE_STANDBY,
 };
 
-enum max77802_opmode {
-	MAX77802_OPMODE_OFF,
-	MAX77802_OPMODE_STANDBY,
-	MAX77802_OPMODE_LP,
-	MAX77802_OPMODE_NORMAL,
-};
-
 struct max77686_opmode_data {
 	int id;
 	int mode;
-- 
2.1.0

[PATCH v2 5/7] regulator: max77802: Document regulator opmode DT properties

[Javier Martinez Canillas]

Some regulators from the max77802 PMIC support to be configured in one
of two operating mode: Output ON (normal) and Output On Low Power Mode.
Not all regulators support these two modes and for some of them, the
mode can be changed while the system is running in normal operation
while others only support their mode to be changed on system suspend.

Extend the max77802 PMIC binding adding Device Tree properties so the
regulators operating modes can be configured.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

Changes since v1: None

 .../devicetree/bindings/regulator/max77802.txt     | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/max77802.txt b/Documentation/devicetree/bindings/regulator/max77802.txt
index 5aeaffc..34812e0 100644
--- a/Documentation/devicetree/bindings/regulator/max77802.txt
+++ b/Documentation/devicetree/bindings/regulator/max77802.txt
@@ -25,6 +25,39 @@ with their hardware counterparts as follow. The valid names are:
 			example: LDO1, LDO2, LDO35.
 	-BUCKn 	:	for BUCKs, where n can lie in range 1 to 10.
 			example: BUCK1, BUCK5, BUCK10.
+
+Besides the standard regulator constraints, the max77802 regulator supports
+two different operating modes: Normal and Low Power Mode. Some regulators
+support the modes to be changed at startup or by the consumers during normal
+operation while others only support to change the mode during system suspend.
+The following optional properties can be used to configure the operating modes:
+
+- maxim,regulator-initial-mode: initial operating mode.
+  This property can only be used on regulators that support changing their mode
+  during normal operation. These regulators are LDO1, LDO3, LDO20 and LDO21.
+- maxim,regulator-disk-mode: operating mode for the regulator when the system
+  enters in the Suspend-to-Disk state.
+- maxim,regulator-mem-mode: operating mode for the regulator when the system
+  enters in the Suspend-to-RAM state.
+
+The value for maxim,regulator-[initial,disk,mem]-mode is one of the following:
+	1: Normal regulator voltage output mode.
+	3: Low Power which reduces the quiescent current down to only 1uA
+
+The list of valid modes are defined in the dt-bindings/regulator/regulator.h
+header and can be included by device tree source files. If no mode is defined,
+then the OS will not manage the modes and the HW default values will be used.
+
+The maxim,regulator-[initial,disk,mem]-mode properties can only be used with
+regulators that support changing their mode to Low Power Mode during suspend.
+These regulators are BUCKs 2-4 and LDOs 1-35.
+
+The maxim,regulator-[disk,mem]-mode property only takes effect if the regulator
+has been marked as enabled for the given suspend mode using the standard
+"regulator-on-in-suspend" property. If the regulator has not been explicitly
+enabled or if it was marked as disabled with "regulator-off-in-suspend", then
+setting the operating mode for that state will have no effect.
+
 Example:
 
 	max77802@09 {
@@ -36,11 +69,23 @@ Example:
 		#size-cells = <0>;
 
 		regulators {
+			ldo1_reg: LDO1 {
+				regulator-name = "vdd_1v0";
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-always-on;
+				maxim,regulator-initial-mode = <MAX77802_OPMODE_LP>;
+			};
+
 			ldo11_reg: LDO11 {
 				regulator-name = "vdd_ldo11";
 				regulator-min-microvolt = <1900000>;
 				regulator-max-microvolt = <1900000>;
 				regulator-always-on;
+				maxim,regulator-mem-mode = <MAX77802_OPMODE_LP>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			buck1_reg: BUCK1 {
-- 
2.1.0

[PATCH v2 6/7] regulator: max77802: Parse regulator operating mode properties

[Javier Martinez Canillas]

The max77802 PMIC regulators output can be configured in one of two
modes: Output ON (normal) and Output ON in Low Power Mode. Some of
the regulators support their operating mode to be changed on startup
or by consumers when the system is running while others only support
their operating mode to be changed while the system has entered in a
suspend state.

The max77802 PMIC Device Tree binding document a set of properties to
configure the regulators operating modes from a FDT. This patch parse
those properties and fills the regulator constraints so the regulator
core can call the suspend handlers when the system enters into sleep.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

Changes since v1:
 - Use the static inline max77802_map_mode() function instead of a macro.
   Suggested by Mark Brown.

 drivers/regulator/max77802.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
index 5839c45..e2fbd45 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -518,6 +518,22 @@ static struct regulator_desc regulators[] = {
 };
 
 #ifdef CONFIG_OF
+
+static void max77802_parse_opmodes(struct device_node *np,
+				   struct regulation_constraints *cons)
+{
+	u32 pval;
+
+	if (!of_property_read_u32(np, "maxim,regulator-initial-mode", &pval))
+		cons->initial_mode = max77802_map_mode(pval);
+
+	if (!of_property_read_u32(np, "maxim,regulator-disk-mode", &pval))
+		cons->state_disk.mode = max77802_map_mode(pval);
+
+	if (!of_property_read_u32(np, "maxim,regulator-mem-mode", &pval))
+		cons->state_mem.mode = max77802_map_mode(pval);
+}
+
 static int max77802_pmic_dt_parse_pdata(struct platform_device *pdev,
 					struct max77686_platform_data *pdata)
 {
@@ -555,6 +571,8 @@ static int max77802_pmic_dt_parse_pdata(struct platform_device *pdev,
 		rdata[i].initdata = rmatch.init_data;
 		rdata[i].of_node = rmatch.of_node;
 		rdata[i].id = regulators[i].id;
+		max77802_parse_opmodes(rdata[i].of_node,
+				       &rdata[i].initdata->constraints);
 	}
 
 	pdata->regulators = rdata;
-- 
2.1.0

[PATCH v2 7/7] ARM: dts: Configure regulators for suspend on exynos Peach boards

[Javier Martinez Canillas]

The regulator core now has support to choose if a regulator
has to be enabled or disabled during system suspend and also
the max77802 driver supports changing the regulator operating
mode during runtime and when the system enters into sleep mode.

To lower power during suspend, configure the regulators state
using the same configuration found in the ChromeOS 3.8 kernel.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---

Changes since v1: None

 arch/arm/boot/dts/exynos5420-peach-pit.dts | 81 ++++++++++++++++++++++++++++++
 arch/arm/boot/dts/exynos5800-peach-pi.dts  | 81 ++++++++++++++++++++++++++++++
 2 files changed, 162 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 9a050e1..72d4e14 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -13,6 +13,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/clock/maxim,max77802.h>
+#include <dt-bindings/regulator/maxim,max77802.h>
 #include "exynos5420.dtsi"
 
 / {
@@ -192,6 +193,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck2_reg: BUCK2 {
@@ -201,6 +205,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck3_reg: BUCK3 {
@@ -210,6 +217,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck4_reg: BUCK4 {
@@ -219,6 +229,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck5_reg: BUCK5 {
@@ -227,6 +240,9 @@
 				regulator-max-microvolt = <1200000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck6_reg: BUCK6 {
@@ -236,6 +252,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck7_reg: BUCK7 {
@@ -244,6 +263,9 @@
 				regulator-max-microvolt = <1350000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			buck8_reg: BUCK8 {
@@ -252,6 +274,9 @@
 				regulator-max-microvolt = <2850000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck9_reg: BUCK9 {
@@ -260,6 +285,9 @@
 				regulator-max-microvolt = <2000000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			buck10_reg: BUCK10 {
@@ -268,6 +296,9 @@
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo1_reg: LDO1 {
@@ -275,6 +306,10 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				maxim,regulator-mem-mode = <MAX77802_OPMODE_LP>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo2_reg: LDO2 {
@@ -288,6 +323,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				maxim,regulator-mem-mode = <MAX77802_OPMODE_LP>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			vqmmc_sdcard: ldo4_reg: LDO4 {
@@ -295,6 +334,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <2800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo5_reg: LDO5 {
@@ -302,6 +344,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo6_reg: LDO6 {
@@ -309,6 +354,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo7_reg: LDO7 {
@@ -322,6 +370,9 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo9_reg: LDO9 {
@@ -329,6 +380,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				maxim,regulator-mem-mode = <MAX77802_OPMODE_LP>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo10_reg: LDO10 {
@@ -336,6 +391,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo11_reg: LDO11 {
@@ -343,6 +401,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				maxim,regulator-mem-mode = <MAX77802_OPMODE_LP>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo12_reg: LDO12 {
@@ -350,6 +412,9 @@
 				regulator-min-microvolt = <3000000>;
 				regulator-max-microvolt = <3000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo13_reg: LDO13 {
@@ -357,6 +422,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				maxim,regulator-mem-mode = <MAX77802_OPMODE_LP>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo14_reg: LDO14 {
@@ -364,6 +433,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo15_reg: LDO15 {
@@ -371,6 +443,9 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo17_reg: LDO17 {
@@ -378,6 +453,9 @@
 				regulator-min-microvolt = <900000>;
 				regulator-max-microvolt = <1400000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo18_reg: LDO18 {
@@ -451,6 +529,9 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo32_reg: LDO32 {
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index e8fdda8..f64a7bf 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -13,6 +13,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/clock/maxim,max77802.h>
+#include <dt-bindings/regulator/maxim,max77802.h>
 #include "exynos5800.dtsi"
 
 / {
@@ -191,6 +192,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck2_reg: BUCK2 {
@@ -200,6 +204,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck3_reg: BUCK3 {
@@ -209,6 +216,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck4_reg: BUCK4 {
@@ -218,6 +228,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck5_reg: BUCK5 {
@@ -226,6 +239,9 @@
 				regulator-max-microvolt = <1200000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck6_reg: BUCK6 {
@@ -235,6 +251,9 @@
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-ramp-delay = <12500>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck7_reg: BUCK7 {
@@ -243,6 +262,9 @@
 				regulator-max-microvolt = <1350000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			buck8_reg: BUCK8 {
@@ -251,6 +273,9 @@
 				regulator-max-microvolt = <2850000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			buck9_reg: BUCK9 {
@@ -259,6 +284,9 @@
 				regulator-max-microvolt = <2000000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			buck10_reg: BUCK10 {
@@ -267,6 +295,9 @@
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
 				regulator-boot-on;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo1_reg: LDO1 {
@@ -274,6 +305,10 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				maxim,regulator-mem-mode = <MAX77802_OPMODE_LP>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo2_reg: LDO2 {
@@ -287,6 +322,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				maxim,regulator-mem-mode = <MAX77802_OPMODE_LP>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			vqmmc_sdcard: ldo4_reg: LDO4 {
@@ -294,6 +333,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <2800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo5_reg: LDO5 {
@@ -301,6 +343,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo6_reg: LDO6 {
@@ -308,6 +353,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo7_reg: LDO7 {
@@ -321,6 +369,9 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo9_reg: LDO9 {
@@ -328,6 +379,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				maxim,regulator-mem-mode = <MAX77802_OPMODE_LP>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo10_reg: LDO10 {
@@ -335,6 +390,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo11_reg: LDO11 {
@@ -342,6 +400,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				maxim,regulator-mem-mode = <MAX77802_OPMODE_LP>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo12_reg: LDO12 {
@@ -349,6 +411,9 @@
 				regulator-min-microvolt = <3000000>;
 				regulator-max-microvolt = <3000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo13_reg: LDO13 {
@@ -356,6 +421,10 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				maxim,regulator-mem-mode = <MAX77802_OPMODE_LP>;
+				regulator-state-mem {
+					regulator-on-in-suspend;
+				};
 			};
 
 			ldo14_reg: LDO14 {
@@ -363,6 +432,9 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo15_reg: LDO15 {
@@ -370,6 +442,9 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo17_reg: LDO17 {
@@ -377,6 +452,9 @@
 				regulator-min-microvolt = <900000>;
 				regulator-max-microvolt = <1400000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo18_reg: LDO18 {
@@ -450,6 +528,9 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-always-on;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			ldo32_reg: LDO32 {
-- 
2.1.0

Re: [PATCH v2 4/7] regulator: max77802: Add header for operating modes

[Lee Jones]

On Thu, 16 Oct 2014, Javier Martinez Canillas wrote:

> Add a header file for the max77802 constants that could be shared between
> the regulator driver and Device Tree source files. Also, remove standby
> and off opmodes since only normal and low power are valid operating modes.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
> 
> Changes since v1: None
> 
>  drivers/regulator/max77802.c                   |  1 +
>  include/dt-bindings/regulator/maxim,max77802.h | 18 ++++++++++++++++++
>  include/linux/mfd/max77686.h                   |  7 -------

Acked-by: Lee Jones <lee.jones@linaro.org>

>  3 files changed, 19 insertions(+), 7 deletions(-)
>  create mode 100644 include/dt-bindings/regulator/maxim,max77802.h
> 
> diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
> index 3abf99d..5839c45 100644
> --- a/drivers/regulator/max77802.c
> +++ b/drivers/regulator/max77802.c
> @@ -33,6 +33,7 @@
>  #include <linux/regulator/of_regulator.h>
>  #include <linux/mfd/max77686.h>
>  #include <linux/mfd/max77686-private.h>
> +#include <dt-bindings/regulator/maxim,max77802.h>
>  
>  /* Default ramp delay in case it is not manually set */
>  #define MAX77802_RAMP_DELAY		100000		/* uV/us */
> diff --git a/include/dt-bindings/regulator/maxim,max77802.h b/include/dt-bindings/regulator/maxim,max77802.h
> new file mode 100644
> index 0000000..cf28631
> --- /dev/null
> +++ b/include/dt-bindings/regulator/maxim,max77802.h
> @@ -0,0 +1,18 @@
> +/*
> + * Copyright (C) 2014 Google, Inc
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Device Tree binding constants for the Maxim 77802 PMIC regulators
> + */
> +
> +#ifndef _DT_BINDINGS_REGULATOR_MAXIM_MAX77802_H
> +#define _DT_BINDINGS_REGULATOR_MAXIM_MAX77802_H
> +
> +/* Regulator operating modes */
> +#define MAX77802_OPMODE_LP	1
> +#define MAX77802_OPMODE_NORMAL	3
> +
> +#endif /* _DT_BINDINGS_REGULATOR_MAXIM_MAX77802_H */
> diff --git a/include/linux/mfd/max77686.h b/include/linux/mfd/max77686.h
> index 7e6dc4b..553f7d0 100644
> --- a/include/linux/mfd/max77686.h
> +++ b/include/linux/mfd/max77686.h
> @@ -131,13 +131,6 @@ enum max77686_opmode {
>  	MAX77686_OPMODE_STANDBY,
>  };
>  
> -enum max77802_opmode {
> -	MAX77802_OPMODE_OFF,
> -	MAX77802_OPMODE_STANDBY,
> -	MAX77802_OPMODE_LP,
> -	MAX77802_OPMODE_NORMAL,
> -};
> -
>  struct max77686_opmode_data {
>  	int id;
>  	int mode;

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 4/7] regulator: max77802: Add header for operating modes

[Lee Jones]

On Thu, 16 Oct 2014, Javier Martinez Canillas wrote:

> Add a header file for the max77802 constants that could be shared between
> the regulator driver and Device Tree source files. Also, remove standby
> and off opmodes since only normal and low power are valid operating modes.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez-ZGY8ohtN/8rSCDK34cm6iQ@public.gmane.org.uk>
> ---
> 
> Changes since v1: None
> 
>  drivers/regulator/max77802.c                   |  1 +
>  include/dt-bindings/regulator/maxim,max77802.h | 18 ++++++++++++++++++
>  include/linux/mfd/max77686.h                   |  7 -------

Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

>  3 files changed, 19 insertions(+), 7 deletions(-)
>  create mode 100644 include/dt-bindings/regulator/maxim,max77802.h
> 
> diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
> index 3abf99d..5839c45 100644
> --- a/drivers/regulator/max77802.c
> +++ b/drivers/regulator/max77802.c
> @@ -33,6 +33,7 @@
>  #include <linux/regulator/of_regulator.h>
>  #include <linux/mfd/max77686.h>
>  #include <linux/mfd/max77686-private.h>
> +#include <dt-bindings/regulator/maxim,max77802.h>
>  
>  /* Default ramp delay in case it is not manually set */
>  #define MAX77802_RAMP_DELAY		100000		/* uV/us */
> diff --git a/include/dt-bindings/regulator/maxim,max77802.h b/include/dt-bindings/regulator/maxim,max77802.h
> new file mode 100644
> index 0000000..cf28631
> --- /dev/null
> +++ b/include/dt-bindings/regulator/maxim,max77802.h
> @@ -0,0 +1,18 @@
> +/*
> + * Copyright (C) 2014 Google, Inc
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Device Tree binding constants for the Maxim 77802 PMIC regulators
> + */
> +
> +#ifndef _DT_BINDINGS_REGULATOR_MAXIM_MAX77802_H
> +#define _DT_BINDINGS_REGULATOR_MAXIM_MAX77802_H
> +
> +/* Regulator operating modes */
> +#define MAX77802_OPMODE_LP	1
> +#define MAX77802_OPMODE_NORMAL	3
> +
> +#endif /* _DT_BINDINGS_REGULATOR_MAXIM_MAX77802_H */
> diff --git a/include/linux/mfd/max77686.h b/include/linux/mfd/max77686.h
> index 7e6dc4b..553f7d0 100644
> --- a/include/linux/mfd/max77686.h
> +++ b/include/linux/mfd/max77686.h
> @@ -131,13 +131,6 @@ enum max77686_opmode {
>  	MAX77686_OPMODE_STANDBY,
>  };
>  
> -enum max77802_opmode {
> -	MAX77802_OPMODE_OFF,
> -	MAX77802_OPMODE_STANDBY,
> -	MAX77802_OPMODE_LP,
> -	MAX77802_OPMODE_NORMAL,
> -};
> -
>  struct max77686_opmode_data {
>  	int id;
>  	int mode;

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 5/7] regulator: max77802: Document regulator opmode DT properties

[Mark Brown]

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

On Thu, Oct 16, 2014 at 06:48:51PM +0200, Javier Martinez Canillas wrote:

> +- maxim,regulator-initial-mode: initial operating mode.
> +  This property can only be used on regulators that support changing their mode
> +  during normal operation. These regulators are LDO1, LDO3, LDO20 and LDO21.
> +- maxim,regulator-disk-mode: operating mode for the regulator when the system
> +  enters in the Suspend-to-Disk state.
> +- maxim,regulator-mem-mode: operating mode for the regulator when the system
> +  enters in the Suspend-to-RAM state.

This seems pretty ugly since it's not integrated with the suspend state
binding at all - adding new suspend modes is going to involve changing
the binding which seems icky.  Adding a standard property to set modes
doesn't seem so bad, I think a translation function to parse device
specific mode bindings in properties might be the way forwards.

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

Re: [PATCH v2 5/7] regulator: max77802: Document regulator opmode DT properties

[Javier Martinez Canillas]

Hello Mark,

On 10/17/2014 01:57 PM, Mark Brown wrote:
> On Thu, Oct 16, 2014 at 06:48:51PM +0200, Javier Martinez Canillas wrote:
> 
>> +- maxim,regulator-initial-mode: initial operating mode.
>> +  This property can only be used on regulators that support changing their mode
>> +  during normal operation. These regulators are LDO1, LDO3, LDO20 and LDO21.
>> +- maxim,regulator-disk-mode: operating mode for the regulator when the system
>> +  enters in the Suspend-to-Disk state.
>> +- maxim,regulator-mem-mode: operating mode for the regulator when the system
>> +  enters in the Suspend-to-RAM state.
> 
> This seems pretty ugly since it's not integrated with the suspend state
> binding at all - adding new suspend modes is going to involve changing
> the binding which seems icky.  Adding a standard property to set modes
> doesn't seem so bad, I think a translation function to parse device
> specific mode bindings in properties might be the way forwards.
> 

Just to be sure I understood correctly, are you suggesting something like this?

	ldo1_reg: LDO1 {
		regulator-name = "vdd_1v0";
		regulator-min-microvolt = <1000000>;
		regulator-max-microvolt = <1000000>;
		regulator-state-mem {
			regulator-on-in-suspend;
			regulator-mode = <MAX77802_OPMODE_LP>;
		};
	};

In other words, extending Chanwoo Choi's original suspend state binding to add
the regulator-mode property that was present in his v3 [0] but instead trying
to use the standard REGULATOR_MODE_*, say that each regulator driver should
define it's own device-specific set of modes and a do the translation to fill
standard modes in the struct regulation_constraints {initial,disk,mem} mode?

That way adding new suspend states, will only require changing the generic
regulator binding but not the regulator driver specific bindings.

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/8/13/768

Re: [PATCH v2 5/7] regulator: max77802: Document regulator opmode DT properties

[Javier Martinez Canillas]

Hello Mark,

On 10/17/2014 01:57 PM, Mark Brown wrote:
> On Thu, Oct 16, 2014 at 06:48:51PM +0200, Javier Martinez Canillas wrote:
> 
>> +- maxim,regulator-initial-mode: initial operating mode.
>> +  This property can only be used on regulators that support changing their mode
>> +  during normal operation. These regulators are LDO1, LDO3, LDO20 and LDO21.
>> +- maxim,regulator-disk-mode: operating mode for the regulator when the system
>> +  enters in the Suspend-to-Disk state.
>> +- maxim,regulator-mem-mode: operating mode for the regulator when the system
>> +  enters in the Suspend-to-RAM state.
> 
> This seems pretty ugly since it's not integrated with the suspend state
> binding at all - adding new suspend modes is going to involve changing
> the binding which seems icky.  Adding a standard property to set modes
> doesn't seem so bad, I think a translation function to parse device
> specific mode bindings in properties might be the way forwards.
> 

Just to be sure I understood correctly, are you suggesting something like this?

	ldo1_reg: LDO1 {
		regulator-name = "vdd_1v0";
		regulator-min-microvolt = <1000000>;
		regulator-max-microvolt = <1000000>;
		regulator-state-mem {
			regulator-on-in-suspend;
			regulator-mode = <MAX77802_OPMODE_LP>;
		};
	};

In other words, extending Chanwoo Choi's original suspend state binding to add
the regulator-mode property that was present in his v3 [0] but instead trying
to use the standard REGULATOR_MODE_*, say that each regulator driver should
define it's own device-specific set of modes and a do the translation to fill
standard modes in the struct regulation_constraints {initial,disk,mem} mode?

That way adding new suspend states, will only require changing the generic
regulator binding but not the regulator driver specific bindings.

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/8/13/768
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/7] regulator: max77802: Add .{get,set}_mode callbacks

[Mark Brown]

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

On Thu, Oct 16, 2014 at 06:48:47PM +0200, Javier Martinez Canillas wrote:
> Some max77802 LDOs (1, 3, 20 and 21) support to be configured in Low
> Power Mode during system normal operation. Add function handlers for
> the .get_mode and .set_mode operations to set the mode on these LDOs.

Applied, thanks.

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

Re: [PATCH v2 2/7] regulator: max77802: Add set suspend mode for BUCKs and simplify code

[Mark Brown]

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

On Thu, Oct 16, 2014 at 06:48:48PM +0200, Javier Martinez Canillas wrote:
> The max77802 PMIC has a special enable pin (PWRREQ) that can be used
> by the Application Processor (AP) to power down and up voltage rails.

Applied, thanks.

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

Re: [PATCH v2 3/7] regulator: max77802: Don't treat OFF as an operating mode

[Mark Brown]

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

On Thu, Oct 16, 2014 at 06:48:49PM +0200, Javier Martinez Canillas wrote:
> The only operating modes that are supported by the regulators in the
> max77802 PMIC are Output ON (normal) and Output On in Low Power Mode.
> OFF was wrongly counted as an operating mode while is only a regulator
> status. Make clear in the code that OFF is not an operating mode.

Applied, thanks.

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

Re: [PATCH v2 4/7] regulator: max77802: Add header for operating modes

[Mark Brown]

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

On Thu, Oct 16, 2014 at 06:48:50PM +0200, Javier Martinez Canillas wrote:
> Add a header file for the max77802 constants that could be shared between
> the regulator driver and Device Tree source files. Also, remove standby
> and off opmodes since only normal and low power are valid operating modes.

Applied, thanks.

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

Re: [PATCH v2 5/7] regulator: max77802: Document regulator opmode DT properties

[Mark Brown]

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

On Fri, Oct 17, 2014 at 02:39:15PM +0200, Javier Martinez Canillas wrote:

> Just to be sure I understood correctly, are you suggesting something like this?

> 	ldo1_reg: LDO1 {
> 		regulator-name = "vdd_1v0";
> 		regulator-min-microvolt = <1000000>;
> 		regulator-max-microvolt = <1000000>;
> 		regulator-state-mem {
> 			regulator-on-in-suspend;
> 			regulator-mode = <MAX77802_OPMODE_LP>;
> 		};
> 	};

> In other words, extending Chanwoo Choi's original suspend state binding to add
> the regulator-mode property that was present in his v3 [0] but instead trying
> to use the standard REGULATOR_MODE_*, say that each regulator driver should
> define it's own device-specific set of modes and a do the translation to fill
> standard modes in the struct regulation_constraints {initial,disk,mem} mode?

> That way adding new suspend states, will only require changing the generic
> regulator binding but not the regulator driver specific bindings.

Something like that, yes.  Not sure if numbers or strings are the best
way of doing the mode but it probably doesn't matter too much now we
have preprocessor support for inclue files.

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

Re: [PATCH v2 5/7] regulator: max77802: Document regulator opmode DT properties

[Mark Brown]

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

On Fri, Oct 17, 2014 at 02:39:15PM +0200, Javier Martinez Canillas wrote:

> Just to be sure I understood correctly, are you suggesting something like this?

> 	ldo1_reg: LDO1 {
> 		regulator-name = "vdd_1v0";
> 		regulator-min-microvolt = <1000000>;
> 		regulator-max-microvolt = <1000000>;
> 		regulator-state-mem {
> 			regulator-on-in-suspend;
> 			regulator-mode = <MAX77802_OPMODE_LP>;
> 		};
> 	};

> In other words, extending Chanwoo Choi's original suspend state binding to add
> the regulator-mode property that was present in his v3 [0] but instead trying
> to use the standard REGULATOR_MODE_*, say that each regulator driver should
> define it's own device-specific set of modes and a do the translation to fill
> standard modes in the struct regulation_constraints {initial,disk,mem} mode?

> That way adding new suspend states, will only require changing the generic
> regulator binding but not the regulator driver specific bindings.

Something like that, yes.  Not sure if numbers or strings are the best
way of doing the mode but it probably doesn't matter too much now we
have preprocessor support for inclue files.

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

Re: [PATCH v2 5/7] regulator: max77802: Document regulator opmode DT properties

[Javier Martinez Canillas]

Hello Mark,

On 10/17/2014 03:54 PM, Mark Brown wrote:
> On Fri, Oct 17, 2014 at 02:39:15PM +0200, Javier Martinez Canillas wrote:
> 
>> Just to be sure I understood correctly, are you suggesting something like this?
> 
>> 	ldo1_reg: LDO1 {
>> 		regulator-name = "vdd_1v0";
>> 		regulator-min-microvolt = <1000000>;
>> 		regulator-max-microvolt = <1000000>;
>> 		regulator-state-mem {
>> 			regulator-on-in-suspend;
>> 			regulator-mode = <MAX77802_OPMODE_LP>;
>> 		};
>> 	};
> 
>> In other words, extending Chanwoo Choi's original suspend state binding to add
>> the regulator-mode property that was present in his v3 [0] but instead trying
>> to use the standard REGULATOR_MODE_*, say that each regulator driver should
>> define it's own device-specific set of modes and a do the translation to fill
>> standard modes in the struct regulation_constraints {initial,disk,mem} mode?
> 
>> That way adding new suspend states, will only require changing the generic
>> regulator binding but not the regulator driver specific bindings.
> 
> Something like that, yes.  Not sure if numbers or strings are the best

Perfect will re-spin then, many thanks again for your feedback and suggestions.

> way of doing the mode but it probably doesn't matter too much now we
> have preprocessor support for inclue files.
> 

I usually prefer to avoid strings when possible since a typo can't be
detected when building the DTB and could be hard to debug at runtime while
a typo on a macro will be detected by the preprocessor at build time.

But I don't have a strong opinion either.

Best regards,
Javier

Re: [PATCH v2 5/7] regulator: max77802: Document regulator opmode DT properties

[Javier Martinez Canillas]

Hello Mark,

On 10/17/2014 03:54 PM, Mark Brown wrote:
> On Fri, Oct 17, 2014 at 02:39:15PM +0200, Javier Martinez Canillas wrote:
> 
>> Just to be sure I understood correctly, are you suggesting something like this?
> 
>> 	ldo1_reg: LDO1 {
>> 		regulator-name = "vdd_1v0";
>> 		regulator-min-microvolt = <1000000>;
>> 		regulator-max-microvolt = <1000000>;
>> 		regulator-state-mem {
>> 			regulator-on-in-suspend;
>> 			regulator-mode = <MAX77802_OPMODE_LP>;
>> 		};
>> 	};
> 
>> In other words, extending Chanwoo Choi's original suspend state binding to add
>> the regulator-mode property that was present in his v3 [0] but instead trying
>> to use the standard REGULATOR_MODE_*, say that each regulator driver should
>> define it's own device-specific set of modes and a do the translation to fill
>> standard modes in the struct regulation_constraints {initial,disk,mem} mode?
> 
>> That way adding new suspend states, will only require changing the generic
>> regulator binding but not the regulator driver specific bindings.
> 
> Something like that, yes.  Not sure if numbers or strings are the best

Perfect will re-spin then, many thanks again for your feedback and suggestions.

> way of doing the mode but it probably doesn't matter too much now we
> have preprocessor support for inclue files.
> 

I usually prefer to avoid strings when possible since a typo can't be
detected when building the DTB and could be hard to debug at runtime while
a typo on a macro will be detected by the preprocessor at build time.

But I don't have a strong opinion either.

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html