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
