On 11 December 2014 at 13:42, Tomasz Figa <tfiga at chromium.org> wrote: > Hi Ulf, > > On Thu, Dec 11, 2014 at 8:58 PM, Ulf Hansson <ulf.hansson at linaro.org> wrote: >> On 11 December 2014 at 09:26, Tomasz Figa <tfiga at chromium.org> 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
