On Wed, Dec 04, 2013 at 03:20:10PM +0000, Dave Martin wrote:
> On Mon, Dec 02, 2013 at 04:20:05PM +0000, Lorenzo Pieralisi wrote:
> > ARM based platforms implement a variety of power management schemes that
> > allow processors to enter at run-time low-power states, aka C-states
> > in ACPI jargon. The parameters defining these C-states vary on a 
> > per-platform
> > basis forcing the OS to hardcode the state parameters in platform
> > specific static tables whose size grows as the number of platforms supported
> > in the kernel increases and hampers device drivers standardization.
> > 
> > Therefore, this patch aims at standardizing C-state device tree bindings for
> > ARM platforms. Bindings define C-state parameters inclusive of entry methods
> > and state latencies, to allow operating systems to retrieve the
> > configuration entries from the device tree and initialize the related
> > power management drivers, paving the way for common code in the kernel
> > to deal with power states and removing the need for static data in current
> > and previous kernel versions.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieral...@arm.com>
> > +
> 
> [...]
> 
> > +   - cpus
> 
> Because cpus is really a topology subtree, it might be good to have
> different names.
> 
> To avoid confusion with Mark's affinity properties, maybe a different
> word would be preferable instead of "affinity".
> 
> Maybe "topology" instead of "cpus", and "affected-topology" instead of
> "affinity"?
> 
> 
> cpufreq also has its concepts of "related" and "affected" cpus, which
> tries to describe something similar (in all honesty, I always struggle
> to remember which is which ... but if we were consistent with it, that
> might help).

Yes, I will try to come up with something clearer.

> > +           Usage: Optional
> > +           Value type: <phandle>
> > +           Definition: If defined, the phandle points to a node in the
> > +                       cpu-map[2] representing all CPUs on which C-state
> > +                       is valid. If not present or system is UP, the
> > +                       C-state has to be considered valid for all CPUs in
> > +                       the system.
> > +
> > +   - affinity
> > +           Usage: Optional
> > +           Value type: <phandle>
> > +           Definition: If defined, phandle points to a node in the
> > +                       cpu-map[2] that represents all CPUs that are
> > +                       affected (ie share) by the C-state and have to
> > +                       be coordinated on C-state entry/exit. If not
> > +                       present or system is UP, the C-state is local to
> > +                       a CPU and need no coordination (ie it is a CPU
> > +                       state, that does not require coordination with
> > +                       other CPUs). If present, the affinity property
> > +                       must contain a phandle to a cpu-map node that
> > +                       represents a subset, possibly inclusive of the
> > +                       CPUs described through the cpus property.
> 
> Can you elaborate on how cpus and affinity might be different?

I was referring to:

- cpus -> processor type (eg valid on A15 or A7, or different implementations
  of the same processor in the same chip)
- affinity -> power domain (a subset of the cpus that require coordination)

Nowadays the distinction does not make much sense (I hardly see a power
state valid on eg A15 clusters [cpus], where just a subset of its cpus need to
be coordinated [affinity] - might be if other levels of caches are added or
if you have multiple clusters of the same CPU type with different power
states capabilities).

I think this deserves more attention, and probably adding power domain
information can remove this mumbo jumbo, there is a scary level of
duplicated information in there.

> The statement about "having to be coordainted" also feels a bit vague,
> though I'm not sure how much we can usefully say here.
> 
> If we describe power domains more explicitly it might help with this,
> because that could bring some description of what needs to be
> coordinated.

Yes, see above.

> > +   - power-depth
> > +           Usage: Required
> > +           Value type: <u32>
> > +           Definition: Integer value, starting from 2 (value 0 meaning
> > +                       running and value 1 representing power depth of
> > +                       wfi (C1)), that defines the level of depth of a
> > +                       power state.
> > +                       The system denotes power states with different
> > +                       depths, an increasing value meaning less power
> > +                       consumption and might involve powering down more
> > +                       components.  Devices that are affected by
> > +                       C-states entry must define the maximum power
> > +                       depth supported in their respective device tree
> > +                       bindings so that OSPM can take decision on how
> > +                       to handle the device in question when the C-state
> > +                       is entered. All devices (per-CPU or external) with
> > +                       a power depth lower than the one defined in the
> > +                       C-state entry stop operating when the C-state
> > +                       is entered and action is required by OSPM to
> > +                       guarantee their logic and memory content is saved
> > +                       restored to guarantee proper functioning.
> 
> Any reason to use numbers instead of strings?
> 
> Strings make the DT more readable ... we would presumably only have to
> parse this information once, so it shouldn't be an overhead, unless there
> are hundreds of C-state nodes.

Yes, but it is supposed to be a unique identifier in the entire system.
Ok, we can create a list of strings denoting power depths, as long as
they are "standard" fine by me, but I think that a number would be
easier to use, even though honestly I think it is better to use power
domains and get rid of this property altogether.

Lorenzo

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to