On Wed, Jul 18, 2018 at 6:13 PM, Robin Murphy [off-list ref] wrote:

On 18/07/18 10:30, Vivek Gautam wrote:

quoted

On Wed, Jul 11, 2018 at 3:23 PM, Rafael J. Wysocki [off-list ref]
wrote:

quoted

On Sunday, July 8, 2018 7:34:12 PM CEST Vivek Gautam wrote:

quoted

From: Sricharan R <redacted>

Finally add the device link between the master device and
smmu, so that the smmu gets runtime enabled/disabled only when the
master needs it. This is done from add_device callback which gets
called once when the master is added to the smmu.

Signed-off-by: Sricharan R <redacted>
Signed-off-by: Vivek Gautam <redacted>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Cc: Rafael J. Wysocki <redacted>
Cc: Lukas Wunner <lukas@wunner.de>
---

  - Change since v11
    * Replaced DL_FLAG_AUTOREMOVE flag with DL_FLAG_AUTOREMOVE_SUPPLIER.

  drivers/iommu/arm-smmu.c | 12 ++++++++++++
  1 file changed, 12 insertions(+)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 09265e206e2d..916cde4954d2 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c

@@ -1461,8 +1461,20 @@ static int arm_smmu_add_device(struct device

*dev)

       iommu_device_link(&smmu->iommu, dev);

+     if (pm_runtime_enabled(smmu->dev) &&

Why does the creation of the link depend on whether or not runtime PM
is enabled for the MMU device?

What about system-wide PM and system shutdown?  Are they always
guaranteed
to happen in the right order without the link?

Hi Robin,

As Rafael pointed, we should the device link creation should not depend on
runtime PM being enabled or not, as we would also want to guarantee
that system wide PM callbacks are called in the right order for smmu
and clients.

Does this change of removing the check for pm_runtime_enabled() from here
looks okay to you?

FWIW the existing system PM ops make no claim to be perfect, and I wouldn't
be at all surprised if it was only by coincidence that my devices happened
to put on the relevant lists in the right order to start with. If we no
longer need to worry about explicit device_link housekeeping in the SMMU
driver, then creating them unconditionally sounds like the sensible thing to
do. I'd be inclined to treat failure as non-fatal like we do for the sysfs
link, though, since it's another thing that correct SMMU operation doesn't
actually depend on (at this point we don't necessarily know if this consumer
even has a driver at all).

Thanks. I will then respin the patch taking care of treating failure
as non-fatal.

quoted

FYI, as discussed in the first patch [1] of this series, I will add a
system wide
suspend callback - arm_smmu_pm_suspend, that would do clock disable, and
will
add corresponding clock enable calls in arm_smmu_pm_resume().

OK, I still don't really understand the finer points of how system PM and
runtime PM interact, but if making it robust is just a case of calling the
runtime suspend/resume hooks as appropriate from the system ones, that
sounds reasonable.

Sure. Thanks.

Best regards
Vivek

Robin.

quoted

[1] https://lore.kernel.org/patchwork/patch/960460/


Best regards
Vivek

quoted

quoted

+         !device_link_add(dev, smmu->dev,
+                     DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_SUPPLIER))
{
+             dev_err(smmu->dev, "Unable to add link to the consumer
%s\n",
+                     dev_name(dev));
+             ret = -ENODEV;
+             goto out_unlink;
+     }
+
       return 0;

+out_unlink:
+     iommu_device_unlink(&smmu->iommu, dev);
+     arm_smmu_master_free_smes(fwspec);
  out_cfg_free:
       kfree(cfg);
  out_free:

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno