[PATCH v2 0/3] ARM: rk3288 : Add PM Domain support

Grygorii Strashko [off-list ref] writes:

Hi,

On 10/01/2014 11:25 PM, Kevin Hilman wrote:

quoted

Doug Anderson [off-list ref] writes:

quoted

On Wed, Oct 1, 2014 at 9:51 AM, Kevin Hilman [off-list ref] wrote:

quoted

+Geert, Ulf

"jinkun.hong" [off-list ref] writes:

quoted

From: "jinkun.hong" <redacted>

Use PM Domain framework to manage the clock.

Which clock?  This changelog needs a more thorough description.

I think what he meant was:

Use the generic PM Domain framework for Rockchip

...but I agree that he could describe it more.

quoted

Also,
with this series alone, it's not clear how the power-domain transitions
ever happen, since I don't see any devices hooked up to your power
domains, or do I see your platform using runtime PM.  In order for this
to be reviewed properely, it's important for reviewers to be able to see
how this PM domain support will be used.

I noticed that too.  As the patchset currently stands it only ever disables...

quoted

Also, remember that the DT is supposed to reflect the hardware, not the
design choices of linux drivers.  Because of that, it's a little
surprising to see clocks as properties of a power domain because clocks
are usually properties of devices.

I haven't dug all the way into the hardware to figure out why (or if
this is really necessary), but right now the rockchip power domain
driver only leaves these clocks on during the powering on and powering
off of the power domain.  In other words to turn on the power domain:

1. Turn on all clocks
2. Flip the bit that sets the power domain on
3. Wait until hardware says power domain is on.
4. Turn off all the clocks.

...and to turn off:

1. Turn on all clocks
2. Flip the bit that sets the power domain off
3. Wait until hardware says power domain is off.
2. Turn off all the clocks.

...if the above is actually necessary when turning on and off power
domains then it seems like it is actually describing the hardware.

Yes, I understand. The need to have some *device* clocks enabled when
powering on/off the power-domain itself is quite common across many
SoCs.

My point is that these clocks are actually properties of *devices*, not
the power-domain itself.  In the shmobile example I pointed to, the
clocks are properties of the devices in DT, and the devices are attached
to the powerdomain.  When the devices are connected to the power-domain,
the custom attach function looks up all the *device* clocks and and
addes them to the power-domain using pm_clk_add.

Sorry, but this is not quite right :(
Patches from Geert will add only first clock for device and not all
of them. 
The right way to add all clocks for device from DT using 
generic PM domain represented in my series:
 
 ARM: keystone: pm: switch to use generic pm domains
 http://www.mail-archive.com/linux-kernel at vger.kernel.org/msg735601.html

Thanks for clarifying.

Kevin