[PATCH 0/2] regulator: of: Add support for pasing regulator suspend state

[PATCH 0/2] regulator: of: Add support for pasing regulator suspend state

[Chanwoo Choi]

The regulators would set different state/mode according to the kind of suspend
state. So regulation_constraints structure has already regulator suspend state filed.
This patch parse regulator suspend state from devicetree file.

For example:

	ldoX_reg: LDOx {
		regulator-name = "VAP_XXX_1.2V";
		regulator-min-microvolt = <1200000>;
		regulator-max-microvolt = <1200000>;
		regulator-always-on;

		regulator-initial-state = <3>;	/* PM_SUSPEND_MEM */
		regulator-state-standby {
			regulator-volt = <1200000>;
			regulator-mode = <0x2>;	/* REGULATOR_MODE_NORMAL */
		};

		regulator-state-mem {
			regulator-volt = <1200000>;
			regulator-mode = <0x8>;	/* REGULATOR_MODE_STANDBY */
			regulator-off-in-suspend;
		};

		regulator-state-disk {
			regulator-volt = <1200000>;
			regulator-mode = <0x2>;	/* REGULATOR_MODE_NORMAL */
			regulator-on-in-suspend;
		};
	};

Chanwoo Choi (2):
  regulator: of: Add support for parsing regulator_state for suspend state
  dt-bindings: regulator: Add regulator suspend state for PM state

 .../devicetree/bindings/regulator/regulator.txt    | 12 +++++
 drivers/regulator/of_regulator.c                   | 61 +++++++++++++++++++++-
 2 files changed, 71 insertions(+), 2 deletions(-)

-- 
1.8.0

[PATCH 0/2] regulator: of: Add support for pasing regulator suspend state

[Chanwoo Choi]

The regulators would set different state/mode according to the kind of suspend
state. So regulation_constraints structure has already regulator suspend state filed.
This patch parse regulator suspend state from devicetree file.

For example:

	ldoX_reg: LDOx {
		regulator-name = "VAP_XXX_1.2V";
		regulator-min-microvolt = <1200000>;
		regulator-max-microvolt = <1200000>;
		regulator-always-on;

		regulator-initial-state = <3>;	/* PM_SUSPEND_MEM */
		regulator-state-standby {
			regulator-volt = <1200000>;
			regulator-mode = <0x2>;	/* REGULATOR_MODE_NORMAL */
		};

		regulator-state-mem {
			regulator-volt = <1200000>;
			regulator-mode = <0x8>;	/* REGULATOR_MODE_STANDBY */
			regulator-off-in-suspend;
		};

		regulator-state-disk {
			regulator-volt = <1200000>;
			regulator-mode = <0x2>;	/* REGULATOR_MODE_NORMAL */
			regulator-on-in-suspend;
		};
	};

Chanwoo Choi (2):
  regulator: of: Add support for parsing regulator_state for suspend state
  dt-bindings: regulator: Add regulator suspend state for PM state

 .../devicetree/bindings/regulator/regulator.txt    | 12 +++++
 drivers/regulator/of_regulator.c                   | 61 +++++++++++++++++++++-
 2 files changed, 71 insertions(+), 2 deletions(-)

-- 
1.8.0

--
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

[PATCH 1/2] regulator: of: Add support for parsing regulator_state for suspend state

[Chanwoo Choi]

The regulation_constraints structure includes specific field to support
suspend state for global PMIC STANDBY/HIBERNATE mode. This patch add support
for parsing regulator_state for suspend state.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/regulator/of_regulator.c | 61 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 59 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index ee5e67b..f8925a2 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -16,12 +16,20 @@
 #include <linux/regulator/machine.h>
 #include <linux/regulator/of_regulator.h>
 
+const char *const regulator_states[PM_SUSPEND_MAX + 1] = {
+	[PM_SUSPEND_STANDBY]	= "regulator-state-standby",
+	[PM_SUSPEND_MEM]	= "regulator-state-mem",
+	[PM_SUSPEND_MAX]	= "regulator-state-disk",
+};
+
 static void of_get_regulation_constraints(struct device_node *np,
 					struct regulator_init_data **init_data)
 {
-	const __be32 *min_uV, *max_uV;
+	const __be32 *min_uV, *max_uV, *uV_offset, *suspend_uV;
 	struct regulation_constraints *constraints = &(*init_data)->constraints;
-	int ret;
+	struct regulator_state *suspend_state;
+	struct device_node *suspend_np;
+	int ret, i;
 	u32 pval;
 
 	constraints->name = of_get_property(np, "regulator-name", NULL);
@@ -70,6 +78,55 @@ static void of_get_regulation_constraints(struct device_node *np,
 	ret = of_property_read_u32(np, "regulator-enable-ramp-delay", &pval);
 	if (!ret)
 		constraints->enable_time = pval;
+
+	ret = of_property_read_u32(np, "regulator-initial-state", &pval);
+	if (!ret)
+		constraints->initial_state = pval;
+
+	for (i = 0; i < ARRAY_SIZE(regulator_states); i++) {
+		switch (i) {
+		case PM_SUSPEND_STANDBY:
+			suspend_state = &constraints->state_standby;
+			break;
+		case PM_SUSPEND_MEM:
+			suspend_state = &constraints->state_mem;
+			break;
+		case PM_SUSPEND_MAX:
+			suspend_state = &constraints->state_disk;
+			break;
+		case PM_SUSPEND_ON:
+		case PM_SUSPEND_FREEZE:
+		default:
+			continue;
+		};
+
+		suspend_np = of_get_child_by_name(np, regulator_states[i]);
+		if (!suspend_np || !suspend_state)
+			continue;
+
+		suspend_uV = of_get_property(suspend_np, "regulator-volt", NULL);
+		if (suspend_uV) {
+			suspend_state->uV = be32_to_cpu(*suspend_uV);
+
+			if (suspend_state->uV < constraints->min_uV)
+				suspend_state->uV = constraints->min_uV;
+			if (suspend_state->uV > constraints->max_uV)
+				suspend_state->uV = constraints->max_uV;
+		}
+
+		ret = of_property_read_u32(suspend_np, "regulator-mode", &pval);
+		if (!ret)
+			suspend_state->mode = pval;
+
+		if (of_property_read_bool(suspend_np, "regulator-on-in-suspend"))
+			suspend_state->enabled = true;
+
+		if (of_property_read_bool(suspend_np, "regulator-off-in-suspend"))
+			suspend_state->disabled = true;
+
+		suspend_state = NULL;
+		suspend_np = NULL;
+	}
 }
 
 /**
-- 
1.8.0

Re: [PATCH 1/2] regulator: of: Add support for parsing regulator_state for suspend state

[Mark Brown]

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

On Mon, Jun 09, 2014 at 07:29:39PM +0900, Chanwoo Choi wrote:

> +		ret = of_property_read_u32(suspend_np, "regulator-mode", &pval);
> +		if (!ret)
> +			suspend_state->mode = pval;

If we're going to do that we'd need to define what modes mean in the DT
binding - currently there's no support for modes at all because it's
complicated to come up with sensible and meaningful generic defintions.

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

Re: [PATCH 1/2] regulator: of: Add support for parsing regulator_state for suspend state

[Mark Brown]

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

On Mon, Jun 09, 2014 at 07:29:39PM +0900, Chanwoo Choi wrote:

> +		ret = of_property_read_u32(suspend_np, "regulator-mode", &pval);
> +		if (!ret)
> +			suspend_state->mode = pval;

If we're going to do that we'd need to define what modes mean in the DT
binding - currently there's no support for modes at all because it's
complicated to come up with sensible and meaningful generic defintions.

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

Re: [PATCH 1/2] regulator: of: Add support for parsing regulator_state for suspend state

[Chanwoo Choi]

Hi Mark,

On 06/10/2014 04:21 AM, Mark Brown wrote:
> On Mon, Jun 09, 2014 at 07:29:39PM +0900, Chanwoo Choi wrote:
> 
>> +		ret = of_property_read_u32(suspend_np, "regulator-mode", &pval);
>> +		if (!ret)
>> +			suspend_state->mode = pval;
> 
> If we're going to do that we'd need to define what modes mean in the DT
> binding - currently there's no support for modes at all because it's
> complicated to come up with sensible and meaningful generic defintions.
> 

The 'regulator-mode' of this patch means following defined constant variable
as following in include/linux/regulator/consumer.h:
#define REGULATOR_MODE_FAST		0x1
#define REGULATOR_MODE_NORMAL		0x2
#define REGULATOR_MODE_IDLE		0x4
#define REGULATOR_MODE_STANDBY		0x8

If regulator consumer would use this mode('regulator-mode),
specific regulator driver should implement rdev->desc->ops->set_suspend_mode function
to control regulator state according to 'regulator-mode' in suspend state.

Best Regards,
Chanwoo CHoi

Re: [PATCH 1/2] regulator: of: Add support for parsing regulator_state for suspend state

[Chanwoo Choi]

Hi Mark,

On 06/10/2014 04:21 AM, Mark Brown wrote:
> On Mon, Jun 09, 2014 at 07:29:39PM +0900, Chanwoo Choi wrote:
> 
>> +		ret = of_property_read_u32(suspend_np, "regulator-mode", &pval);
>> +		if (!ret)
>> +			suspend_state->mode = pval;
> 
> If we're going to do that we'd need to define what modes mean in the DT
> binding - currently there's no support for modes at all because it's
> complicated to come up with sensible and meaningful generic defintions.
> 

The 'regulator-mode' of this patch means following defined constant variable
as following in include/linux/regulator/consumer.h:
#define REGULATOR_MODE_FAST		0x1
#define REGULATOR_MODE_NORMAL		0x2
#define REGULATOR_MODE_IDLE		0x4
#define REGULATOR_MODE_STANDBY		0x8

If regulator consumer would use this mode('regulator-mode),
specific regulator driver should implement rdev->desc->ops->set_suspend_mode function
to control regulator state according to 'regulator-mode' in suspend state.

Best Regards,
Chanwoo CHoi



--
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 1/2] regulator: of: Add support for parsing regulator_state for suspend state

[Mark Brown]

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

On Tue, Jun 10, 2014 at 08:59:01AM +0900, Chanwoo Choi wrote:
> On 06/10/2014 04:21 AM, Mark Brown wrote:

> > If we're going to do that we'd need to define what modes mean in the DT
> > binding - currently there's no support for modes at all because it's
> > complicated to come up with sensible and meaningful generic defintions.

> The 'regulator-mode' of this patch means following defined constant variable
> as following in include/linux/regulator/consumer.h:
> #define REGULATOR_MODE_FAST		0x1
> #define REGULATOR_MODE_NORMAL		0x2
> #define REGULATOR_MODE_IDLE		0x4
> #define REGULATOR_MODE_STANDBY		0x8

> If regulator consumer would use this mode('regulator-mode),
> specific regulator driver should implement rdev->desc->ops->set_suspend_mode function
> to control regulator state according to 'regulator-mode' in suspend state.

Sure, but what do those mean?

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

Re: [PATCH 1/2] regulator: of: Add support for parsing regulator_state for suspend state

[Mark Brown]

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

On Tue, Jun 10, 2014 at 08:59:01AM +0900, Chanwoo Choi wrote:
> On 06/10/2014 04:21 AM, Mark Brown wrote:

> > If we're going to do that we'd need to define what modes mean in the DT
> > binding - currently there's no support for modes at all because it's
> > complicated to come up with sensible and meaningful generic defintions.

> The 'regulator-mode' of this patch means following defined constant variable
> as following in include/linux/regulator/consumer.h:
> #define REGULATOR_MODE_FAST		0x1
> #define REGULATOR_MODE_NORMAL		0x2
> #define REGULATOR_MODE_IDLE		0x4
> #define REGULATOR_MODE_STANDBY		0x8

> If regulator consumer would use this mode('regulator-mode),
> specific regulator driver should implement rdev->desc->ops->set_suspend_mode function
> to control regulator state according to 'regulator-mode' in suspend state.

Sure, but what do those mean?

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

Re: [PATCH 1/2] regulator: of: Add support for parsing regulator_state for suspend state

[Chanwoo Choi]

On 06/10/2014 07:02 PM, Mark Brown wrote:
> On Tue, Jun 10, 2014 at 08:59:01AM +0900, Chanwoo Choi wrote:
>> On 06/10/2014 04:21 AM, Mark Brown wrote:
> 
>>> If we're going to do that we'd need to define what modes mean in the DT
>>> binding - currently there's no support for modes at all because it's
>>> complicated to come up with sensible and meaningful generic defintions.
> 
>> The 'regulator-mode' of this patch means following defined constant variable
>> as following in include/linux/regulator/consumer.h:
>> #define REGULATOR_MODE_FAST		0x1
>> #define REGULATOR_MODE_NORMAL		0x2
>> #define REGULATOR_MODE_IDLE		0x4
>> #define REGULATOR_MODE_STANDBY		0x8
> 
>> If regulator consumer would use this mode('regulator-mode),
>> specific regulator driver should implement rdev->desc->ops->set_suspend_mode function
>> to control regulator state according to 'regulator-mode' in suspend state.
> 
> Sure, but what do those mean?
> 

We could check basic explanation of regulator mode in include/linux/regulator/consumer.h.
Should I add detailed descritpion of REGULATOR_MODE_* constant in drivers/regulator/of_regulator.c?

Re: [PATCH 1/2] regulator: of: Add support for parsing regulator_state for suspend state

[Chanwoo Choi]

On 06/10/2014 07:02 PM, Mark Brown wrote:
> On Tue, Jun 10, 2014 at 08:59:01AM +0900, Chanwoo Choi wrote:
>> On 06/10/2014 04:21 AM, Mark Brown wrote:
> 
>>> If we're going to do that we'd need to define what modes mean in the DT
>>> binding - currently there's no support for modes at all because it's
>>> complicated to come up with sensible and meaningful generic defintions.
> 
>> The 'regulator-mode' of this patch means following defined constant variable
>> as following in include/linux/regulator/consumer.h:
>> #define REGULATOR_MODE_FAST		0x1
>> #define REGULATOR_MODE_NORMAL		0x2
>> #define REGULATOR_MODE_IDLE		0x4
>> #define REGULATOR_MODE_STANDBY		0x8
> 
>> If regulator consumer would use this mode('regulator-mode),
>> specific regulator driver should implement rdev->desc->ops->set_suspend_mode function
>> to control regulator state according to 'regulator-mode' in suspend state.
> 
> Sure, but what do those mean?
> 

We could check basic explanation of regulator mode in include/linux/regulator/consumer.h.
Should I add detailed descritpion of REGULATOR_MODE_* constant in drivers/regulator/of_regulator.c?



--
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

[PATCH 2/2] dt-bindings: regulator: Add regulator suspend state for PM state

[Chanwoo Choi]

This patch add regulator suspend state to constraint in dt file. The regulation_
constraints structure already has regulator suspend state field as following.
The regulator suspend state control the state of regulator according to
PM (Power Management) state.
- struct regulator_state state_disk
- struct regulator_state state_mem
- struct regulator_state state_standby

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 Documentation/devicetree/bindings/regulator/regulator.txt | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index e2c7f1e..b2a4a72 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -19,6 +19,12 @@ Optional properties:
   design requires. This property describes the total system ramp time
   required due to the combination of internal ramping of the regulator itself,
   and board design issues such as trace capacitance and load on the supply.
+- regulator-initial-state: initial state for suspend state
+- regulator-state-[standby/mem/disk] sub-root node for suspend state
+	regulator-volt: voltage consumers may set in suspend state
+	regulator-mode: voltage mode in suspend state
+	regulator-on-in-suspend: regulator should be on in suspend state
+	regulator-off-in-suspend: regulator should be off in suspend state
 
 Deprecated properties:
 - regulator-compatible: If a regulator chip contains multiple
@@ -34,6 +40,12 @@ Example:
 		regulator-max-microvolt = <2500000>;
 		regulator-always-on;
 		vin-supply = <&vin>;
+
+		regulator-state-mem {
+			regulator-volt = <1000000>;
+			regulator-mode = <0x8>;
+			regulator-off-in-suspend;
+		};
 	};
 
 Regulator Consumers:
-- 
1.8.0

Re: [PATCH 2/2] dt-bindings: regulator: Add regulator suspend state for PM state

[Mark Brown]

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

On Mon, Jun 09, 2014 at 07:29:40PM +0900, Chanwoo Choi wrote:

> +- regulator-initial-state: initial state for suspend state
> +- regulator-state-[standby/mem/disk] sub-root node for suspend state
> +	regulator-volt: voltage consumers may set in suspend state
> +	regulator-mode: voltage mode in suspend state
> +	regulator-on-in-suspend: regulator should be on in suspend state
> +	regulator-off-in-suspend: regulator should be off in suspend state

These need to define what the states are in general terms - in
particular it's very unclear what standby is and how it differs from
suspend to RAM.  You also need to define what happens if neither
X-in-suspend property is set.

> +		regulator-state-mem {
> +			regulator-volt = <1000000>;
> +			regulator-mode = <0x8>;
> +			regulator-off-in-suspend;
> +		};

Probably best to remove mode from this.

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

Re: [PATCH 2/2] dt-bindings: regulator: Add regulator suspend state for PM state

[Mark Brown]

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

On Mon, Jun 09, 2014 at 07:29:40PM +0900, Chanwoo Choi wrote:

> +- regulator-initial-state: initial state for suspend state
> +- regulator-state-[standby/mem/disk] sub-root node for suspend state
> +	regulator-volt: voltage consumers may set in suspend state
> +	regulator-mode: voltage mode in suspend state
> +	regulator-on-in-suspend: regulator should be on in suspend state
> +	regulator-off-in-suspend: regulator should be off in suspend state

These need to define what the states are in general terms - in
particular it's very unclear what standby is and how it differs from
suspend to RAM.  You also need to define what happens if neither
X-in-suspend property is set.

> +		regulator-state-mem {
> +			regulator-volt = <1000000>;
> +			regulator-mode = <0x8>;
> +			regulator-off-in-suspend;
> +		};

Probably best to remove mode from this.

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

Re: [PATCH 2/2] dt-bindings: regulator: Add regulator suspend state for PM state

[Chanwoo Choi]

Hi Mark,

On 06/10/2014 04:16 AM, Mark Brown wrote:
> On Mon, Jun 09, 2014 at 07:29:40PM +0900, Chanwoo Choi wrote:
> 
>> +- regulator-initial-state: initial state for suspend state
>> +- regulator-state-[standby/mem/disk] sub-root node for suspend state
>> +	regulator-volt: voltage consumers may set in suspend state
>> +	regulator-mode: voltage mode in suspend state
>> +	regulator-on-in-suspend: regulator should be on in suspend state
>> +	regulator-off-in-suspend: regulator should be off in suspend state
> 
> These need to define what the states are in general terms - in
> particular it's very unclear what standby is and how it differs from
> suspend to RAM.  You also need to define what happens if neither
> X-in-suspend property is set.

OK, I'll add more description.

> 
>> +		regulator-state-mem {
>> +			regulator-volt = <1000000>;
>> +			regulator-mode = <0x8>;
>> +			regulator-off-in-suspend;
>> +		};
> 
> Probably best to remove mode from this.
> 

OK, I'll remove it. (regulator-mode)


I modify description as following. Please review this description.

--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -19,6 +19,24 @@ Optional properties:
   design requires. This property describes the total system ramp time
   required due to the combination of internal ramping of the regulator itself,
   and board design issues such as trace capacitance and load on the supply.
+- regulator-initial-state: initial state for suspend state
+- regulator-state-standby sub-root node for Standby mode
+  : the device is in a power-saving state, but can also receive certain events,
+  specific behavior depends on the specific device.
+- regulator-state-mem sub-root node for Suspend-to-RAM mode
+  : suspend to memory, the device goes to sleep, but all data stored in memory,
+  only some external interrupt can wake the device.
+- regulator-state-disk sub-root node for Suspend-to-disk mode
+  : suspend to disk, this state operates similarly to Suspend-to-RAM,
+  but includes a final step of writing memory contents to disk.
+- regulator-state-[standby/mem/disk] node has following common properties:
+	- regulator-volt: voltage consumers may set in suspend state.
+	- regulator-mode: voltage mode in suspend state.
+	- regulator-on-in-suspend: regulator should be on in suspend state.
+	- regulator-off-in-suspend: regulator should be off in suspend state.
+	If node don't include regulator-[on/off]-in-suspend, can't change
+	regulator state in suspend mode and only should sustain the regulator
+	state of normal state.
 
 Deprecated properties:
 - regulator-compatible: If a regulator chip contains multiple
@@ -34,6 +52,11 @@ Example:
 		regulator-max-microvolt = <2500000>;
 		regulator-always-on;
 		vin-supply = <&vin>;
+
+		regulator-state-mem {
+			regulator-volt = <1000000>;
+			regulator-off-in-suspend;
+		};
 	};
 

Best Regards,
Chanwoo Choi