On 08/16/2012 12:08 AM, Alexandre Courbot wrote:Some device drivers (panel backlights especially) need to follow precise
sequences for powering on and off, involving gpios, regulators, PWMs
with a precise powering order and delays to respect between each steps.
These sequences are board-specific, and do not belong to a particular
driver - therefore they have been performed by board-specific hook
functions to far.
With the advent of the device tree and of ARM kernels that are not
board-tied, we cannot rely on these board-specific hooks anymore but
need a way to implement these sequences in a portable manner. This patch
introduces a simple interpreter that can execute such power sequences
encoded either as platform data or within the device tree.
diff --git a/Documentation/devicetree/bindings/power_seq/power_seq.txt b/Documentation/devicetree/bindings/power_seq/power_seq.txt
+Specifying Power Sequences in the Device Tree
+=============================================
+In the device tree, power sequences are specified as sub-nodes of the device
+node and reference resources declared by that device.
+
+For an introduction about runtime interpreted power sequences, see
+Documentation/power/power_seq.txt and include/linux/power_seq.h.
Device tree bindings shouldn't reference Linux documentation; the
bindings are supposed to be OS-agnostic.
+Power Sequences Structure
+-------------------------
+Power sequences are sub-nodes that are named such as the device driver can find
+them. The driver's documentation should list the sequence names it recognizes.
That's a little roundabout. That might be better as simply:
Valid power sequence names are defined by each device's binding. For a
power sequence named "foo", a node named "foo-power-sequence" defines
that sequence.
+Inside a power sequence node are sub-nodes that describe the different steps
+of the sequence. Each step must be named sequentially, with the first step
+named step0, the second step1, etc. Failure to follow this rule will result in a
+parsing error.
Node names shouldn't be interpreted by the code; nodes should all be
named after the type of object the represent. Hence, every step should
be named just "step" for example.
The node's unit address (@0) should be used to distinguish the nodes.
This requires reg properties within each node to match the unit address,
and hence #address-cells and #size-cells properties in the power
sequence itself.
Note that this somewhat conflicts with accessing the top-level power
sequence by name too; perhaps that should be re-thought too. I must
admit this DT rule kinda sucks.
+Power Sequences Steps
+---------------------
+Every step of a sequence describes an action to be performed on a resource. It
+generally includes a property named after the resource type, and which value
+references the resource to be used. Depending on the resource type, additional
+properties can be defined to control the action to be performed.
I think you need to add a property that indicates what type of resource
each step applies to. Sure, this is implicit in that if a "gpio"
property exists, the step is a GPIO step, but in order to make that
work, you'd have to search each node for each possible resource type's
property name. It'd be far better to read a single type="gpio" property,
then parse the step based on that.
+Example
+-------
+Here are example sequences declared within a backlight device that use all the
+supported resources types:
+
+ backlight {
+ compatible = "pwm-backlight";
+ ...
+
+ /* resources used by the sequences */
+ pwms = <&pwm 2 5000000>;
+ pwm-names = "backlight";
+ power-supply = <&backlight_reg>;
+ enable-gpio = <&gpio 28 0>;
+
+ power-on-sequence {
+ step0 {
+ regulator = "power";
+ enable;
+ };
+ step1 {
+ delay = <10000>;
+ };
+ step2 {
+ pwm = "backlight";
+ enable;
+ };
+ step3 {
+ gpio = "enable";
+ enable;
+ };
+ };
+
+ power-off-sequence {
+ step0 {
+ gpio = "enable";
+ disable;
+ };
+ step1 {
+ pwm = "backlight";
+ disable;
+ };
+ step2 {
+ delay = <10000>;
+ };
+ step3 {
+ regulator = "power";
+ disable;
+ };
+ };
+ };
I notice that for clocks, pwms, and interrupts, the initial step of the
lookup is via a single property that lists all know resources of that
type. Regulators and GPIOs don't follow this style though. Using the
same mechanism for power-sequences would yield something like the
following, which would avoid the "node names must be significant" issue
I mention above, although it does make everything rather more wordy.
[start my proposal]backlight {[end my proposal]
compatible = "pwm-backlight";
/* resources used by the sequences */
pwms = <&pwm 2 5000000>;
pwm-names = "backlight";
power-supply = <&backlight_reg>;
bl-enable-gpio = <&gpio 28 0>;
pwr-seqs = <&bl_on &bl_off>;
pwr-seq-names = "on", "off";
#address-cells = <1>;
#size-cells = <0>;
bl_on: pwr-seq@0 {
reg = <0>;
#address-cells = <1>;
#size-cells = <0>;
step@0 {
reg = <0>;
type = "regulator";
regulator = "power";
enable;
};
step@1 {
reg = <1>;
type = "delay";
delay = <10000>;
};
step@2 {
reg = <2>;
type = "pwm";
pwm = "backlight";
enable;
};
step@3 {
reg = <3>;
type = "gpio";
gpio = "bl-enable";
enable;
};
};
bl_off: pwr-seq@1 {
reg = <1>;
#address-cells = <1>;
#size-cells = <0>;
step@0 {
reg = <0>;
type = "gpio";
gpio = "bl-enable";
disable;
};
step@1 {
reg = <1>;
type = "pwm";
pwm = "backlight";
disable;
};
step@2 {
reg = <2>;
type = "delay";
delay = <10000>;
};
step@3 {
reg = <3>;
type = "regulator";
regulator = "power";
disable;
};
};
};
diff --git a/Documentation/power/power_seq.txt b/Documentation/power/power_seq.txt
+Usage by Drivers and Resources Management
+-----------------------------------------
+Power sequences make use of resources that must be properly allocated and
+managed. The power_seq_build() function builds a power sequence from the
+platform data. It also takes care of resolving and allocating the resources
+referenced by the sequence if needed:
+
+ struct power_seq *power_seq_build(struct device *dev, struct list_head *ress,
+ struct platform_power_seq *pseq);
+
+The 'dev' argument is the device in the name of which the resources are to be
+allocated.
+
+The 'ress' argument is a list to which the resolved resources are appended. This
+avoids allocating a resource referenced in several power sequences multiple
+times.
I see in other parts of the thread, there has been discussion re:
needing the separate ress parameter, and requiring the driver to pass
this in to multiple power_seq_build calls.
The way the pinctrl subsystem solved this was to have a single function
that parsed all pinctrl setting (c.f. all power sequences) at once, and
return a single handle. Later, the driver passes this handle
pinctrl_lookup_state(), along with the requested state (c.f. sequence
name) to search for, and finally passes that handle to
pinctrl_select_state(). Doing something similar here would result in:
struct power_seqs *power_seq_get(struct device *dev);
void power_seq_put(struct power_seqs *seqs);
struct power_seq *power_seq_lookup(struct power_seqs *seqs,
const char *name);
int power_seq_executestruct power_seq *seq);
struct power_seqs *devm_power_seq_get(struct device *dev);
void devm_power_seq_put(struct power_seqs *seqs);
+On success, the function returns a devm allocated resolved sequence that is
Perhaps the function should be named devm_power_seq_build(), in order to
make this obvious to people who only read the client code, and not this
documentation.