[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5017D5C9.3070905@firmworks.com>
Date: Tue, 31 Jul 2012 20:55:37 +0800
From: Mitch Bradley <wmb@...mworks.com>
To: Thierry Reding <thierry.reding@...onic-design.de>
CC: Alex Courbot <acourbot@...dia.com>,
"linux-fbdev@...r.kernel.org" <linux-fbdev@...r.kernel.org>,
Stephen Warren <swarren@...dia.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Mark Brown <broonie@...nsource.wolfsonmicro.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Rob Herring <rob.herring@...xeda.com>,
"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
"devicetree-discuss@...ts.ozlabs.org"
<devicetree-discuss@...ts.ozlabs.org>
Subject: Re: [RFC][PATCH v3 1/3] runtime interpreted power sequences
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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists