| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-id: <27224826.584141389879802275.JavaMail.weblogic@epml20>
Date: Thu, 16 Jan 2014 13:43:24 +0000 (GMT)
From: Saurabh Singh <saurabh1.s@...sung.com>
To: Mark Rutland <mark.rutland@....com>
Cc: "lgirdwood@...il.com" <lgirdwood@...il.com>,
"broonie@...nel.org" <broonie@...nel.org>,
"grant.likely@...aro.org" <grant.likely@...aro.org>,
"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"celinux-dev@...e.celinuxforum.org"
<celinux-dev@...e.celinuxforum.org>,
SREEVATSA D B <srevatsa@...sung.com>,
Praveen BP <bp.praveen@...sung.com>
Subject: Re: [PATCH] Parse missing regulator constraints from device tree blob
Hi Mark,
New patch incorporating your suggestions is given below.
> * 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.
Done. The documentation is in the patch below.
> * It leaks Linux internal details (e.g. suspend_state_t values,
> valid_mode_mask) without any attempt at abstraction, in violation of
> dt principles.
Unlike valid_ops_mask, valid_mode_mask cannot be derived from the other settings.
It depends on the hardware (regulator) capability. So, it has to be specified in DT blob.
> * Accessors are used poorly. Endianness conversion is done manually
> rather than being left to accessors, and property lengths aren't
> checked.
Used the accessors for u32 and bool.
> 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.
This is not input supply. This is operating voltage to be set when device suspends.
diffstat for this patch is:
Documentation/devicetree/bindings/regulator/regulator.txt | 19 ++++++
drivers/regulator/of_regulator.c | 41 ++++++++++++++
2 files changed, 60 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 <saurabh1.s@...sung.com>
--------------------------------------------------------------------------------
diff -uprN -X linux-3.12.6-vanilla/Documentation/dontdiff linux-3.12.6-vanilla/Documentation/devicetree/bindings/regulator/regulator.txt linux-3.12.6/Documentation/devicetree/bindings/regulator/regulator.txt
--- linux-3.12.6-vanilla/Documentation/devicetree/bindings/regulator/regulator.txt 2013-12-20 21:21:33.000000000 +0530
+++ linux-3.12.6/Documentation/devicetree/bindings/regulator/regulator.txt 2014-01-16 18:47:17.708608811 +0530
@@ -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 -uprN -X linux-3.12.6-vanilla/Documentation/dontdiff linux-3.12.6-vanilla/drivers/regulator/of_regulator.c linux-3.12.6/drivers/regulator/of_regulator.c
--- linux-3.12.6-vanilla/drivers/regulator/of_regulator.c 2013-12-20 21:21:33.000000000 +0530
+++ linux-3.12.6/drivers/regulator/of_regulator.c 2014-01-16 18:45:44.135928414 +0530
@@ -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_constraint
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);
}
/**
Powered by blists - more mailing lists