diff mbox

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

Message ID 1413478133-2577-6-git-send-email-javier.martinez@collabora.co.uk
State Needs Review / ACK, archived
Headers show

Checks

Context Check Description
robh/checkpatch warning total: 1 errors, 0 warnings, 0 lines checked
robh/patch-applied success

Commit Message

Javier Martinez Canillas Oct. 16, 2014, 4:48 p.m. UTC
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(+)

Comments

Mark Brown Oct. 17, 2014, 11:57 a.m. UTC | #1
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.
Javier Martinez Canillas Oct. 17, 2014, 12:39 p.m. UTC | #2
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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Oct. 17, 2014, 1:54 p.m. UTC | #3
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.
Javier Martinez Canillas Oct. 17, 2014, 2:18 p.m. UTC | #4
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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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 {