Re: [RFC PATCH 2/2] iommu: rockchip: Handle system-wide and runtime PM

From: Ulf Hansson
Date: Thu Dec 11 2014 - 10:23:05 EST


On 11 December 2014 at 13:42, Tomasz Figa <tfiga@xxxxxxxxxxxx> wrote:
> Hi Ulf,
>
> On Thu, Dec 11, 2014 at 8:58 PM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>> On 11 December 2014 at 09:26, Tomasz Figa <tfiga@xxxxxxxxxxxx> wrote:
>>> This patch modifies the rockchip-iommu driver to consider state of
>>> the power domain the IOMMU is located in. When the power domain
>>> is powered off, the IOMMU cannot be accessed and register programming
>>> must be deferred until the power domain becomes enabled.
>
> [snip]
>
>>> @@ -988,11 +1107,28 @@ static int rk_iommu_probe(struct platform_device *pdev)
>>> return -ENXIO;
>>> }
>>>
>>> + pm_runtime_no_callbacks(dev);
>>> + pm_runtime_enable(dev);
>>> +
>>> + /* Synchronize state of the domain with driver data. */
>>> + pm_runtime_get_sync(dev);
>>> + iommu->is_powered = true;
>>
>> Doesn't the runtime PM status reflect the value of "is_powered", thus
>> why do you need to have a copy of it? Could it perpahps be that you
>> try to cope with the case when CONFIG_PM is unset?
>>
>
> It's worth noting that this driver fully relies on status of other
> devices in the power domain the IOMMU is in and does not enforce the
> status on its own. So in general, as far as my understanding of PM
> runtime subsystem, the status of the IOMMU device will be always
> suspended, because nobody will call pm_runtime_get() on it (except the
> get and put pair in probe). So is_powered is here to track status of
> the domain, not the device. Feel free to suggest a better way, though.

I see, thanks for clarifying. I think it makes sense as is, I have no
better suggestion.

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/