Hi Will,
On 10/24/2012 04:31 AM, Will Deacon wrote:
> Hi Jon,
>
> On Tue, Oct 23, 2012 at 09:31:08PM +0100, Jon Hunter wrote:
>> Commit 7be2958 (ARM: PMU: Add runtime PM Support) updated the ARM PMU code to
>> use runtime PM which was prototyped and validated on the OMAP devices. In this
>> commit, there is no call pm_runtime_enable() and for OMAP devices
>> pm_runtime_enable() is currently being called from the OMAP PMU code when the
>> PMU device is created. However, there are two problems with this:
>>
>> 1. For any other ARM device wishing to use runtime PM for PMU they will need
>> to call pm_runtime_enable() for runtime PM to work.
>> 2. When booting with device-tree and using device-tree to create the PMU
>> device, pm_runtime_enable() needs to be called from within the ARM PERF
>> driver as we are no longer calling any device specific code to create the
>> device. Hence, PMU does not work on OMAP devices that use the runtime PM
>> callbacks when using device-tree to create the PMU device.
>>
>> Therefore, add a new platform data variable to indicate whether runtime PM is
>> used and if so call pm_runtime_enable() when the PMU device is registered. Note
>> that devices using runtime PM may not use the runtime_resume/suspend callbacks
>> and so we cannot use the presence of these handlers in the platform data to
>> determine whether we should call pm_runtime_enable().
>
> [...]
>
>> diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h
>> index a26170d..50a6c3b 100644
>> --- a/arch/arm/include/asm/pmu.h
>> +++ b/arch/arm/include/asm/pmu.h
>> @@ -36,6 +36,7 @@
>> struct arm_pmu_platdata {
>> irqreturn_t (*handle_irq)(int irq, void *dev,
>> irq_handler_t pmu_handler);
>> + bool use_runtime_pm;
>> int (*runtime_resume)(struct device *dev);
>> int (*runtime_suspend)(struct device *dev);
>
> 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.
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.
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.
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
