Grygorii Strashko <grygorii.strashko@xxxxxx> writes: > Hi, > > On 10/01/2014 11:25 PM, Kevin Hilman wrote: >> Doug Anderson <dianders@xxxxxxxxxxxx> writes: >> >>> >>> 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. > > 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@xxxxxxxxxxxxxxx/msg735601.html Thanks for clarifying. 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