[PATCH 2/6] ARM: OMAP3/4: iommu: adapt to runtime pm

[omar.luna@linaro.org (Omar Ramirez Luna)]

On 15 October 2012 22:23, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>> On probe this patch does pm_runtime_enable, however this doesn't mean
>> the device is turned ON or resumed or kept ON all the time.
>
> In fact it's the other way around; pm_runtime_enable turns off the
> power (if it's ON).

pm_runtime_enable just decrements the disable_depth counter. Doesn't
turn off anything by itself.

>>>>>> @@ -1009,7 +1001,8 @@ static int __devexit omap_iommu_remove(struct platform_device *pdev)
>>>>>>         release_mem_region(res->start, resource_size(res));
>>>>>>         iounmap(obj->regbase);
>>>>>>
>>>>>> -       clk_put(obj->clk);
>>>>>> +       pm_runtime_disable(obj->dev);
>>>>>
>>>>> This will turn on the device unnecessarily, wasting power, and there's
>>>>> no need for that, kfree will take care of that without resuming.
>>>>
>>>> Left aside the aesthetics of having balanced calls, the device will be
>>>> enabled if there was a pending resume to be executed, otherwise it
>>>> won't, kfree won't increment the disable_depth counter and I don't
>>>> think that freeing the pointer is enough reason to ignore
>>>> pm_runtime_disable.
>>>
>>> You are doing __pm_runtime_disable(dev, true), kfree will do
>>> __pm_runtime_disable(dev, false), which is what we want. Both will
>>> decrement the disable_depth.
>>
>> I'm quite confused here, could you please point me to the kfree snip
>> that does __pm_runtime_disable(dev, false)?
>
> Sorry, not kfree, but the device removal process:
>
> device_del
>  device_pm_remove
>   pm_runtime_remove
>    __pm_runtime_disable <- HERE

I'm not entirely convinced _iommu_ follows this path, it doesn't
create any devices nor register them... whereas mailbox does create
devices and mailbox does follow this path for the devices created
which are child devices.

So this path won't take care of the omap-iommu device pm_runtime_disable.

>>> But at least you agree that there's a chance that the device will be waken up.
>>
>> Of course, if there is a pending resume to be executed, it must honor
>> that resume request and then turn off the device before removing the
>> iommu, IMHO.
>
> Who will turn off the device afterwards?

What I meant is that omap_iommu_detach should turn off the device
before removing the iommu.

Cheers,

Omar