Doug Anderson <dianders@xxxxxxxxxxxx> writes: > Kevin, > > On Wed, Oct 1, 2014 at 9:51 AM, Kevin Hilman <khilman@xxxxxxxxxx> wrote: >> +Geert, Ulf >> >> "jinkun.hong" <jinkun.hong@xxxxxxxxxxxxxx> writes: >> >>> From: "jinkun.hong" <jinkun.hong@xxxxxxxxxxxxxx> >>> >>> 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. > > >> 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... > > >> 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. Since the clocks are properties of devices, then the pm_clk infrastructure can be used (as the shmobile example shows) so that the SoC specific pm-domain doesn't have to manually walk all the clocks, but instead can just use pm_clk_suspend/pm_clk_resume. Kevin -- 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