[PATCH RFC v2 2/2] Documentation: arm: define DT C-states bindings

[Vincent Guittot]

On 24 January 2014 18:58, Lorenzo Pieralisi <lorenzo.pieralisi at arm.com> wrote:
> Hi Vincent,
>
> On Fri, Jan 24, 2014 at 08:40:40AM +0000, Vincent Guittot wrote:
>
> [...]
>
>> Hi Lorenzo,
>>
>> Sorry for the late reply,
>>
>>
>> > I had an idea. To simplify things, I think that one possibility is to
>> > add a parameter to the power domain specifier (platform specific, see
>> > Tomasz bindings):
>>
>> We can't use a simple boolean state (on/off) for defining the
>> powerdomain state associated to a c-state so your proposal of being
>> able to add a parameter that will define the power domain state is
>> interesting.
>>
>> >
>> > Documentation/devicetree/bindings/power/power_domain.txt
>> >
>> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-January/224928.html
>> >
>> > to represent, when that state is entered the behavior of the power
>> > controller (ie cache RAM retention or cache shutdown or in general any
>> > substate within a power domain). Since it is platform specific, and since
>> > we are able to link caches to the power domain, the power controller will
>> > actually define what happens to the cache when that state is entered
>> > (basically we use the power domain specifier additional parameter to define
>> > a "substate" in that power domain e.g.:
>> >
>> > Example:
>> >
>> > foo_power_controller {
>> >         [...]
>> >         /*
>> >          * first cell is register index, second one is the state index
>> >          * that in turn implies the state behavior - eg cache lost or
>> >          * retained
>> >          */
>> >         #power-domain-cells = <2>;
>> > };
>> >
>> > l1-cache {
>> >         [...]
>> >         /*
>> >          * syntax: power-domains = list of power domain specifiers
>> >                 <[&power_domain_phandle register-index state],[&power_domain_phandle register-index state]>;
>> >                 The syntax is defined by the power controller du jour
>> >                 as described by Tomasz bindings
>> >         */
>> >         power-domains =<&foo_power_controller 0 0 &foo_power_controller 0 1>;
>>
>> Normally, power-domains describes a list of power domain specifiers
>> that are necessary for the l1-cache to at least retain its state so
>> i'm not sure understand your example above
>
>>
>> If we take the example of system that support running, retention and
>> powerdown state described as state 0, 1 and 2 for the power domain, i
>> would have set the l1-cache like:
>>        power-domains =<&foo_power_controller 0 1>;
>>
>> for saying that the state is retained up to state 1
>>
>> Please look below, i have modified the rest of your example accordingly
>>
>> >
>> > }:
>> >
>> > and then
>> >
>> > state0 {
>> >         index = <2>;
>> >         compatible = "arm,cpu-power-state";
>> >         latency = <...>;
>> >         /*
>> >          * This means that when the state is entered, the power
>> >          * controller should use register index 0 and state 0,
>> >          * whose meaning is power controller specific. Since we
>> >          * know all components affected (for every component
>> >          * we declare its power domain(s) and states so we
>> >          * know what components are affected by the state entry.
>> >          * Given the cache node above and this phandle, the state
>> >          * implies that the cache is retained, register index == 0 state == 0
>> >          /*
>> >         power-domain =<&foo_power_controller 0 0>;
>>
>> for retention state we need to set the power domain in state 1
>>         power-domain =<&foo_power_controller 0 1>;
>>
>> > };
>> >
>> > state1 {
>> >         index = <3>;
>> >         compatible = "arm,cpu-power-state";
>> >         latency = <...>;
>> >         /*
>> >          * This means that when the state is entered, the power
>> >          * controller should use register index 0 and state 1,
>> >          * whose meaning is power controller specific. Since we
>> >          * know all components affected (for every component
>> >          * we declare its power domain(s) and states so we
>> >          * know what components are affected by the state entry.
>> >          * Given the cache node above and this phandle, the state
>> >          * implies that the cache is lost, register index == 0 state == 1
>> >          /*
>> >         power-domain =<&foo_power_controller 0 1>;
>>
>> for power down mode, we need to set thge power domain in state 2
>>         power-domain =<&foo_power_controller 0 2>;
>
> Ok, what I meant was not what you got, but your approach looks sensible
> too. What I do not like is that the power-domain specifier is power

sorry for the misconception of your example

> controller specific (that was true even for my example). In theory
> we can achieve something identical by forcing every component in a power
> domain to specify the max C-state index that allows it to retain its

I'm not sure that we should force a component to set an opaque (for
the component) max c-state. The device should describe its power
domain requirements and the correlation of the latter with the
description of the c-state binding should be enough to deduct the max
c-state.

> state (through a specific property). Same logic to your example applies.
> Nice thing is that we do not change the power domain specifiers, bad thing
> is that it adds two properties to each device (c-state index and
> power-domain-specifier - but we can make it hierarchical so that device
> nodes can inherit the maximum operating C-state by inheriting the value
> from a parent node providing a common value).
>
> In my example the third parameter was just a number that the power
> controller would decode (eg 0 = cache retained, 1 = cache lost)
> according to its implementation, it was not a "state index". The
> power controller would know what to do with eg a cache component (that
> declares to be in that power domain) when a C-state with that power
> domain specifier was entered.
>
> Not very different from what you are saying, let's get to the nub:
>
> - Either we define it in a platform specific way through the power
>   domain specifier
> - Or we force a max-c-state-supported property for every device,
>   possibly hierarchical

As explained above, adding a max-cstate property for a device that
only know the power-domain is not a good thing IMHO.

Vincent

>
> Thoughts ?
>
> Thank you !
> Lorenzo
>