diff mbox

Re: [PATCH] Parse missing regulator constraints from device tree blob

Message ID 8217085.584601389885483146.JavaMail.weblogic@epml20
State Superseded, archived
Headers show

Commit Message

Saurabh Singh Jan. 16, 2014, 3:18 p.m. UTC
Hi Mark,

> Please send patches using the process in SubmittingPatches, the formatting

> of the submission is very important for tools like git am which people use to

> work with patches.


As per your request sending the patch and description in git format.
Please find below the patch.

Regards,
Saurabh Singh Sengar
Lead Engineer
Samsung R&D Institute
India

-- 
1.7.0.4

Comments

Mark Brown Jan. 16, 2014, 5:56 p.m. UTC | #1
On Thu, Jan 16, 2014 at 03:18:03PM +0000, Saurabh Singh wrote:
> Hi Mark,
> 
> > Please send patches using the process in SubmittingPatches, the formatting
> > of the submission is very important for tools like git am which people use to
> > work with patches.
> 
> As per your request sending the patch and description in git format.
> Please find below the patch.
> 
> Regards,

No, please do as I asked and follow the process in SubmittingPatches -
as I said the format things are sent in is very important for the
tooling.  Pasting the patch into a mail after some other text definitely
doesn't give a mail in the format covered in SubmittingPatches.  If in
doubt send the mail to yourself and then compare it with other patches
sent to the list and test by applying with git am and make sure the
patch and changelog come out OK.

> +- regulator-valid-modes-mask: valid operations for regulator on particular machine

This is not adequately documented, what are "valid operations" and how
would they be encoded?

> +- regulator-input-uv: regulator input voltage, only if supply is another regulator

Why provide a property for this, surely if there is another regulator we
can just find out from that regulator what voltage it is outputting?

> +- regulator-initial-mode: default mode to set on startup

It is not documented what a mode is here or how one would specify it in
the property.

> +- regulator-initial-state: suspend state to set at init

Again, no semantics are provided for this.

> +- regulator-state-mem, regulator-state-disk, regulator-state-standby:
> +	defines regulator suspend to memory, suspend to disk (hibernate) and standby respectively.
> +	have following sub-constarints:
> +	- regulator-state-uv: suspend voltage
> +	- regulator-state-mode: suspend regulator operating mode
> +	- regulator-state-enabled: is regulator enabled in this suspend state
> +	- regulator-state-disabled: is the regulator disbled in this suspend state

I am very nervous about the idea of putting this stuff into DT.  This
matches less and less well with modern system designs which are becoming
more and more dynamic, and of course the concepts of suspending to
memory, disk and standby are unclear and fluid - what is the difference
between memory and standby for example?

I'd be interested to know if there are real systems that need this and
can't figure out what to do dynamically.
diff mbox

Patch

=================================================================================
From Saurabh Singh <saurabh1.s@samsung.com> Mon Sep 17 00:00:00 2001
Date: Thu, 16 Jan 2014 20:36:15 +0530
Subject: [PATCH] From: Saurabh Singh Sengar <saurabh1.s@samsung.com>

This patch adds support for parsing following regulator contraints from
device tree blob.
1. valid modes mask (valid_modes_mask)
2. input microvolt(input_uV)
3. initial mode (initial_mode)
4. initial state (initial_state)
5. state mem (state_mem)
6. state disk (state_disk)
7. state standby (state_standby)

Signed-off-by: Saurabh Singh Sengar <saurabh1.s@samsung.com>
---
 .../devicetree/bindings/regulator/regulator.txt    |   19 +++++++++
 drivers/regulator/of_regulator.c                   |   41 ++++++++++++++++++++
 2 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index 2bd8f09..7793445 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -14,6 +14,17 @@  Optional properties:
 - regulator-ramp-delay: ramp delay for regulator(in uV/uS)
   For hardwares which support disabling ramp rate, it should be explicitly
   intialised to zero (regulator-ramp-delay = <0>) for disabling ramp delay.
+- regulator-valid-modes-mask: valid operations for regulator on particular machine
+- regulator-input-uv: regulator input voltage, only if supply is another regulator
+- regulator-initial-mode: default mode to set on startup
+- regulator-initial-state: suspend state to set at init
+- regulator-state-mem, regulator-state-disk, regulator-state-standby:
+	defines regulator suspend to memory, suspend to disk (hibernate) and standby respectively.
+	have following sub-constarints:
+	- regulator-state-uv: suspend voltage
+	- regulator-state-mode: suspend regulator operating mode
+	- regulator-state-enabled: is regulator enabled in this suspend state
+	- regulator-state-disabled: is the regulator disbled in this suspend state
 
 Deprecated properties:
 - regulator-compatible: If a regulator chip contains multiple
@@ -29,6 +40,14 @@  Example:
 		regulator-max-microvolt = <2500000>;
 		regulator-always-on;
 		vin-supply = <&vin>;
+		regulator-valid-modes-mask = <REGULATOR_MODE_FAST>;
+		regulator-initial-mode = <REGULATOR_MODE_STANDBY>;
+		regulator-initial-state = <PM_SUSPEND_MEM>;
+		regulator-state-mem {
+			regulator-state-mode = <REGULATOR_MODE_IDLE>;
+			regulator-state-enabled;
+		};
+
 	};
 
 Regulator Consumers:
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 7827384..328d76f 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -16,11 +16,27 @@ 
 #include <linux/regulator/machine.h>
 #include <linux/regulator/of_regulator.h>
 
+/**
+ * set_regulator_state_constraints - set regulator state for low power system states
+ * @np: device node for the low power regulator state
+ * @regulator_state: regulator_state structure need to be filled
+ */
+static void set_regulator_state_constraints(struct device_node *np,
+		struct regulator_state *state)
+{
+	of_property_read_u32(np, "regulator-state-uv", &state->uV);
+	of_property_read_u32(np, "regulator-state-mode", &state->mode);
+	state->enabled = of_property_read_bool(np, "regulator-state-enabled");
+	state->disabled = of_property_read_bool(np, "regulator-state-disabled");
+}
+
+
 static void of_get_regulation_constraints(struct device_node *np,
 					struct regulator_init_data **init_data)
 {
 	const __be32 *min_uV, *max_uV, *uV_offset;
 	const __be32 *min_uA, *max_uA, *ramp_delay;
+	struct device_node *state;
 	struct property *prop;
 	struct regulation_constraints *constraints = &(*init_data)->constraints;
 
@@ -73,6 +89,31 @@  static void of_get_regulation_constraints(struct device_node *np,
 		else
 			constraints->ramp_disable = true;
 	}
+
+	of_property_read_u32(np, "regulator-valid-modes-mask",
+					&constraints->valid_modes_mask);
+	of_property_read_u32(np, "regulator-input-uv",
+					&constraints->input_uV);
+	of_property_read_u32(np, "regulator-initial-mode",
+					&constraints->initial_mode);
+	of_property_read_u32(np, "regulator-initial-state",
+					&constraints->initial_state);
+
+	/* regulator state during low power system states */
+	state = of_find_node_by_name(np, "regulator-state-mem");
+	if (state)
+		set_regulator_state_constraints(state,
+				&constraints->state_mem);
+
+	state = of_find_node_by_name(np, "regulator-state-disk");
+	if (state)
+		set_regulator_state_constraints(state,
+				&constraints->state_disk);
+
+	state = of_find_node_by_name(np, "regulator-state-standby");
+	if (state)
+		set_regulator_state_constraints(state,
+				&constraints->state_standby);
 }
 
 /**