Re: [RFC][PATCH v3 1/3] runtime interpreted power sequences

[Thierry Reding <thierry.reding-RM9K5IK7kjKj5M59NBduVrNAH6kLmebB@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3825 bytes --]

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  &reg                 1
		    3  &pwm  0 5000000      1>;

	power-off = <3  &pwm  0 5000000      0
		     2  &reg                 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?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]