Parse missing regulator constraints from device tree blob

PATCH In review Saurabh Singh 2 participants 2 messages Jan 16, 2014 Voltage and current regulator framework

Share & tools

Saurabh Singh

author

2014-01-16 06:34 UTC #0 lore raw

PATCH 1/1 Parse missing regulator constraints from device tree blob

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)

This patch is currently against a linux 3.12.6 kernel.

diffstat for this patch is:

 of_regulator.c |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

To apply the patch, in the root of a kernel tree use:
patch -p1 < of_regulator.patch

Please let me know any feedback you have on this patch or the approach used.

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

Signed-off-by Saurabh Singh Sengar <[email protected]>

--------------------------------------------------------------------------------

drivers/regulator/of_regulator.c renamed from drivers/regulator/of_regulator.c.orig +63 -0

@@ -16,11 +16,40 @@
 #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 *regulator_state)
+{
+	const __be32 *uV, *mode;
+
+	uV = of_get_property(np, "regulator-state-uV", NULL);
+	if (uV)
+		regulator_state->uV = be32_to_cpu(*uV);
+
+	mode = of_get_property(np, "regulator-state-mode", NULL);
+	if (mode)
+		regulator_state->mode = be32_to_cpu(*mode);
+
+	if (of_find_property(np, "regulator-state-enabled", NULL))
+		regulator_state->enabled = true;
+
+	if (of_find_property(np, "regulator-state-disabled", NULL))
+		regulator_state->disabled = true;
+}
+
+
 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;
+	const __be32  *initial_mode, *initial_state;
+	const __be32  *input_uV, *valid_modes_mask;
+	struct device_node *state;
 	struct property *prop;
 	struct regulation_constraints *constraints = &(*init_data)->constraints;

@@ -73,6 +102,40 @@ static void of_get_regulation_constraint
 		else
 			constraints->ramp_disable = true;
 	}
+
+	valid_modes_mask = of_get_property(np,
+			"regulator-valid-modes-mask", NULL);
+	if (valid_modes_mask)
+		constraints->valid_modes_mask = be32_to_cpu(*valid_modes_mask);
+
+	input_uV = of_get_property(np, "regulator-input-microvolt", NULL);
+	if (input_uV)
+		constraints->input_uV = be32_to_cpu(*input_uV);
+
+	initial_mode = of_get_property(np, "regulator-initial-mode", NULL);
+	if (initial_mode)
+		constraints->initial_mode = be32_to_cpu(*initial_mode);
+
+	initial_state = of_get_property(np, "regulator-initial-state", NULL);
+	if (initial_state)
+		constraints->initial_state = be32_to_cpu(*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);
+
 }
 
 /**

Mark Rutland

2014-01-16 10:28 UTC #1 lore raw

Hi,

Quoted 11 lines · Saurabh Singh

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)
This patch is currently against a linux 3.12.6 kernel.

Quoted 8 lines

diffstat for this patch is:
 of_regulator.c |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)
To apply the patch, in the root of a kernel tree use:
patch -p1 < of_regulator.patch
Please let me know any feedback you have on this patch or the approach used.

There are several issues with this patch:

* None of these properties are documented. Documentation is required so that the contract is defined. That allows people to learn how to use the properties, and makes clear what we can and cannot change kernel-side.

* It leaks Linux internal details (e.g. suspend_state_t values, valid_mode_mask) without any attempt at abstraction, in violation of dt principles.

* Accessors are used poorly. Endianness conversion is done manually rather than being left to accessors, and property lengths aren't checked.

Quoted 19 lines

Regards,
=====================
Saurabh Singh Sengar
Lead Engineer
Samsung R&D Institute
India
Samsung
=====================
Signed-off-by Saurabh Singh Sengar <[email protected]>
--------------------------------------------------------------------------------
drivers/regulator/of_regulator.c renamed from drivers/regulator/of_regulator.c.orig +1 -0
@@ -16,11 +16,40 @@
 #include <linux/regulator/machine.h>
 #include <linux/regulator/of_regulator.h>
 
+/**

drivers/regulator/of_regulator.c line 29

Quoted 10 lines

drivers/regulator/of_regulator.c renamed from drivers/regulator/of_regulator.c.orig +10 -0

+ * 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 *regulator_state)
+{
+	const __be32 *uV, *mode;
+
+	uV = of_get_property(np, "regulator-state-uV", NULL);

Typically properties are all lower-case.

drivers/regulator/of_regulator.c line 31

Quoted 2 lines

drivers/regulator/of_regulator.c renamed from drivers/regulator/of_regulator.c.orig +2 -0
+	if (uV)
+		regulator_state->uV = be32_to_cpu(*uV);

As an example, to use accessors correctly, here you should have:

	u32 uv;
	of_property_read_u32(np, "regulator-state-uv", &uv);

However, as far as I can see this value should come from an input supply anyway.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html