[+ Laurent Pinchart]

Tomasz Figa [off-list ref] writes:

On Thu, Dec 11, 2014 at 8:58 PM, Ulf Hansson [off-list ref] wrote:

[...]

quoted

quoted

@@ -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 still don't like these notifiers.  I think they add ways to bypass
having proper runtime PM implemented for devices/subsystems.

From a high-level, the IOMMU is just another device inside the PM

domain, so ideally it should be doing it's own _get() and _put() calls
so the PM domain code would just do the right thing without the need for
notifiers.

No knowing a lot about the IOMMU API, I'm guessing the reason you're not
doing that is because the IOMMU API currently doesn't have an easy way
to keep track of *active* users so it's not obvious where to put those
_get and _put calls.  If that doesn't exist, perhaps a simple
iommu_get() and iommu_put() API needs to be introduced (which inside the
IOMMU core would just do runtime PM calls) so that users of the IOMMU
could inform the subsystem that the IOMMU is needed and it should not be
powered off.

I Cc'd Laurent because I know he's thought about this before from the
IOMMU side, and not sure if he came up with a solution.

Kevin