Hi Tomasz,

Thanks for your reply.

On 01/17/2018 02:20 PM, Tomasz Figa wrote:
On Tue, Jan 16, 2018 at 10:25 PM, Jeffy Chen <[email protected]> wrote:
When the power domain is powered off, the IOMMU cannot be accessed and
register programming must be deferred until the power domain becomes
enabled.

Add runtime PM support, and use runtime PM device link from IOMMU to
master to startup and shutdown IOMMU.
[snip]
@@ -875,28 +889,19 @@ static size_t rk_iommu_unmap(struct iommu_domain *domain, 
unsigned long _iova,

  static struct rk_iommu *rk_iommu_from_dev(struct device *dev)
  {
-       return dev->archdata.iommu;
+       struct rk_iommudata *data = dev->archdata.iommu;
+
+       return data ? data->iommu : NULL;
  }

Is this change intentionally added to this patch? I see this
potentially relevant for the previous patch in this series.
right, will do that.


[snip]
+static int rk_iommu_startup(struct rk_iommu *iommu)
  {
-       struct rk_iommu *iommu;
+       struct iommu_domain *domain = iommu->domain;
         struct rk_iommu_domain *rk_domain = to_rk_domain(domain);
-       unsigned long flags;
         int ret, i;

-       /*
-        * Allow 'virtual devices' (e.g., drm) to attach to domain.
-        * Such a device does not belong to an iommu group.
-        */
-       iommu = rk_iommu_from_dev(dev);
-       if (!iommu)
-               return 0;
-
-       if (iommu->domain)
-               rk_iommu_detach_device(iommu->domain, dev);
-
         ret = rk_iommu_enable_clocks(iommu);
         if (ret)
                 return ret;


Don't we need to check here (and in _shutdown() too) if we have a
domain attached?
hmmm, right, the startup might been called by resume, so should check iommu->domain here.

but the shutdown would be called at the end of detach or suspend, it could be not attached or attached.

+       mutex_lock(&iommu->pm_mutex);
         ret = rk_iommu_enable_stall(iommu);
         if (ret)
-               goto err_disable_clocks;
+               goto err_unlock_mutex;
[snip]
+       iommu->domain = NULL;
+
+       spin_lock_irqsave(&rk_domain->iommus_lock, flags);
+       list_del_init(&iommu->node);
+       spin_unlock_irqrestore(&rk_domain->iommus_lock, flags);
+
+       if (pm_runtime_get_if_in_use(iommu->dev) > 0) {

Actually, if the above call returns -EINVAL, don't we still need to
call rk_iommu_shutdown(), because it just means runtime PM is disabled
and the IOMMU is always powered on?
ok, will fix that.

+               rk_iommu_shutdown(iommu);
+               pm_runtime_put(iommu->dev);
+       }
+}
[snip]
+       spin_lock_irqsave(&rk_domain->iommus_lock, flags);
+       list_add_tail(&iommu->node, &rk_domain->iommus);
+       spin_unlock_irqrestore(&rk_domain->iommus_lock, flags);
+
+       if (pm_runtime_get_if_in_use(iommu->dev) <= 0)

Ditto.

+               return 0;
+
+       ret = rk_iommu_startup(iommu);
+       if (ret)
+               rk_iommu_detach_device(data->domain, dev);
[snip]
@@ -1108,7 +1175,9 @@ static int rk_iommu_of_xlate(struct device *dev,
                 return -ENODEV;
         }

-       dev->archdata.iommu = platform_get_drvdata(iommu_dev);
+       data->iommu = platform_get_drvdata(iommu_dev);
+       dev->archdata.iommu = data;
+

I think this change might be mistakenly squashed to this patch instead
of previous.
right, will do.

Best regards,
Tomasz





_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to