On 7/31/2012 8:38 PM, Thierry Reding wrote:On Tue, Jul 31, 2012 at 08:22:17PM +0800, Mitch Bradley wrote:On 7/31/2012 6:56 PM, Thierry Reding wrote:On Tue, Jul 31, 2012 at 07:32:20PM +0900, Alex Courbot wrote:On 07/31/2012 07:45 AM, Stephen Warren wrote:I wonder if using the same structure/array as input and output would
simplify the API; the platform data would fill in the fields mentioned
above, and power_seq_build() would parse those, then set other fields in
the same structs to the looked-up handle values?
The thing is that I am not sure what happens to the platform data
once probe() is done. Isn't it customary to mark it with __devinit
and have it freed after probing is successful?
No, platform data should stay around forever. Otherwise, consider what
would happen if your driver is built as a module and you unload and load
it again.
More generally, I think it is a good practice to have data
structures tailored right for what they need to do - code with
members that are meaningful only at given points of an instance's
life tends to be more confusing.
I agree. Furthermore the driver unload/reload would be another reason
not to reuse platform data as the output of the build() function.
But maybe what Stephen meant was more like filling a structure with data
taken from the platform data and pass that to a resolve() function which
would fill in the missing pieces like pointers to actual resources. I
imagine a managed interface would become a little trickier to do using
such an approach.
If the nodes have a unit address (i.e. end in "@n"), which they will
have to if all named "step" and there's more than one of them, then they
will need a matching reg property. Equally, the parent node will need
#address-cells and #size-cells too. So, the last couple lines would be:
power-on-sequence {
#address-cells = <1>;
#size-cells = <0>;
step@0 {
reg = <0>;
That's precisely what I would like to avoid - I don't need the steps
to be numbered and I certainly have no use for a reg property. Isn't
there a way to make it simpler?
It's not technically valid to not have the reg property. Or
#address-cells and #size-cells properties for that matter.
I'm not keen on this representation where individual steps are nodes.
That seems like it could end up being too "heavyweight" for a long sequence.
The other alternative would involve using a single property to encode
one sequence. I think that was the initial proposal, though using proper
phandle encoding it could probably be enhanced a bit. However anything
that involves a single property has the problem that we need to encode
the type of resource as an integer, and that makes things very hard to
read.
So it would look something like this:
power-on = <1 &gpio 6 0 1
0 10000
2 ® 1
3 &pwm 0 5000000 1>;
power-off = <3 &pwm 0 5000000 0
2 ® 0
0 10000
1 &gpio 6 0 0>;
So the first cell would encode the type:
0: delay
1: gpio
2: regulator
3: PWM
The next n cells would be the phandle and the specifier, while the last
cell would encode a resource-specific parameter:
delay: time in microseconds
gpio: set level (0: low, 1: high)
regulator: 0: disable, 1: enable
pwm: 0: disable, 1: enable
I guess this would be more compact, but it is also very hard to read. Is
that something you would be happier with? Perhaps you were thinking of
something completely different?
Perhaps a compact/flexible encoding could be designed, with a textual
encoding that is easy to read. A separate tool could convert the text
encoding to the integer format, annotated with comments containing
the "source text". A file containing that output could be #included
into the dts file.