On 10/24/2012 09:32 AM, Will Deacon wrote: > On Wed, Oct 24, 2012 at 03:16:43PM +0100, Jon Hunter wrote: >> Hi Will, >> >> On 10/24/2012 04:31 AM, Will Deacon wrote: >>> Can we not just use the presence of the resume/suspend function pointers to >>> indicate whether we should enable runtime pm or not? i.e. if they're not >>> NULL, then enable the thing? >> >> I wondered if you would ask that :-) >> >> Unfortunately, we can't. For example, OMAP3430 and OMAP4460 both require >> runtime_pm to be enabled to turn on the debug sub-system in OMAP for PMU >> to work. However, neither of these devices are using the suspend/resume >> callbacks because the HWMOD framework is doing this for us behind the >> scenes. >> >> So then you ask, why do we need the resume/suspend callbacks, well we >> will need them for OMAP4430 where in addition to turning on the debug >> sub-system we need to configure the CTI module. Therefore, I had to add >> another plat data variable. > > Hmmm, now I start to wonder whether your original idea of having separate > callbacks for enable/disable irq and resume/suspend doesn't make more sense. > Then the CTI magic can go in the irq management code and be totally separate > to the PM stuff. > > What do you reckon? The resume/suspend calls really replaced the enable/disable irq callbacks. That still seems like a good approach given that we need runtime PM for OMAP and PMU. The problem is going to be with device-tree. When booting with device-tree we will not call the current OMAP specific code that is creating the PMU device and calling pm_runtime_enable(). So calling pm_runtime_enable() needs to be done somewhere in the PERF driver and that's the real problem here. >> By the way, I did add a comment in the above changelog about this. See >> the last paragraph, not sure if this is 100% clear. > > Sorry, I missed that this morning. > >> One alternative I had thought about was always calling >> pm_runtime_enable() on registering the PMU device and avoid having a >> use_runtime_pm variable. For ARM platforms that don't use runtime PM the >> pm_runtime_enable() function does nothing. However, I was more concerned >> about adding unnecessary overhead for ARM platforms that are using >> runtime PM but don't need runtime PM for PMU. I don't see any side >> affects on our OMAP2 platform that does not need runtime PM for PMU. >> However, was not sure what is best here. > > Nah, we should be able to fix this in the platdata, I'd just rather have > function pointers instead of state variables in there. Well, we could pass a pointer to pm_runtime_enable() function in the platdata. Cheers Jon -- 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