[PATCH] ARM: PMU: fix runtime PM enable

[Jon Hunter]

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