Jon Hunter <jon-hunter@xxxxxx> writes: > Hi Kevin, > > On 05/31/2012 05:36 PM, Kevin Hilman wrote: >> Jon Hunter <jon-hunter@xxxxxx> writes: >> >>> Hi Kevin, >>> >>> On 05/31/2012 03:42 PM, Kevin Hilman wrote: >>>> Jon Hunter <jon-hunter@xxxxxx> writes: >>>> >>>>> Hi Kevin, Will, >>>>> >>>>> On 05/30/2012 08:29 PM, Will Deacon wrote: >>>>>> Hi Kevin, >>>>>> >>>>>> On Wed, May 30, 2012 at 10:50:01PM +0100, Kevin Hilman wrote: >>>>>>> Basically, I don't like the result when we have to hack around missing >>>>>>> runtime PM support for a driver, so IMO, the driver should be updated. >>>>>>> >>>>>>> IOW, it looks to me like the armpmu driver should grow runtime PM >>>>>>> support. The current armpmu_release|reserve should probably be replaced >>>>>>> with runtime PM get/put, and the functionality in those functions would >>>>>>> be the runtime PM callbacks instead. >>>>>>> >>>>>>> Will, any objections to armpmu growing runtime PM support? >>>>>> >>>>>> My plan for the armpmu reservation is to kill the global reservation scheme >>>>>> that we currently have and push those function pointers into the arm_pmu, >>>>>> so that fits with what you'd like. >>>>>> >>>>>> The only concern I have is that we need the mutual exclusion even when we >>>>>> don't have support for runtime PM. If we can solve that then I'm fine with >>>>>> the approach. >>>>> >>>>> To add a bit more food for thought, I had implemented a quick patch to >>>>> add runtime PM support for PMU. You will notice that I have been >>>>> conservative on where I have placed the pm_runtime_get/put calls, >>>>> because I am not too familiar with the PMU driver to know exactly >>>>> where we need to maintain the PMU context. So right now these are just >>>>> around the reserve_hardware/release_hardware calls. This works on OMAP >>>>> for some quick testing. However, I would need to make sure this does >>>>> not break compilation without runtime PM enabled. >>>>> >>>>> Let me know your thoughts. >>>> >>>> That looks good, but I'm curious what would be done in the new >>>> plat->runtime_* hooks. Maybe the irq enable/disable stuff in the pmu >>>> driver needs to be moved into the runtime PM hooks? >>> >>> For omap4, the plat->runtime_* hooks look like ... >>> >>> +static int omap4_pmu_runtime_resume(struct device *dev) >>> +{ >>> + /* configure CTI0 for PMU IRQ routing */ >>> + cti_unlock(&omap4_cti[0]); >>> + cti_map_trigger(&omap4_cti[0], 1, 6, 2); >>> + cti_enable(&omap4_cti[0]); >>> + >>> + /* configure CTI1 for PMU IRQ routing */ >>> + cti_unlock(&omap4_cti[1]); >>> + cti_map_trigger(&omap4_cti[1], 1, 6, 3); >>> + cti_enable(&omap4_cti[1]); >>> + >>> + return 0; >>> +} >>> + >>> +static int omap4_pmu_runtime_suspend(struct device *dev) >>> +{ >>> + cti_disable(&omap4_cti[0]); >>> + cti_disable(&omap4_cti[1]); >>> + >>> + return 0; >>> +} >>> >>> This is what I have implemented so far and currently testing. So really >>> just using the hooks to configure the cross triggering interface. >>> >>> Is this what you were thinking? >>> >> >> Basically, yes. >> >> But since I haven't studied the PMU driver closely, I have some dumb >> questions. My concern is that these look bsically like the >> plat->irq_[enable|disable] hooks, so I guess the root of my question is >> do we need both the irq enable/disable and runtime suspend/resume hooks >> in plat? or can we get by with one set. > > No you are right. The way it is now we could get by with just the one of > hooks. However, the main reason I added the new hooks would be if there > are other places we can call the pm_runtime_* functions. I am not too > familiar with the flow in which the functions are called in the PMU > driver and so this was a simple attempt to push the PM runtime framework > in the PMU driver. > > Hmmm ... however, now looking at the history behind the plat->irq_* > hooks, I see that Ming specifically added these for omap4 [1]. I was > under the impression other architectures may be using this. I guess not. > So if it is preferred we could do-away with the plat->irq_* and replace > with the plat->runtime_*. Yes, that would certainly be my preference from a runtime PM readability PoV. I guess it's Will's call since it's his code. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html