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.pieralisi@xxxxxxx> > > + > > [...] > > > + - 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html