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. > > Thierry > -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html