Hi Robin, On 2/2/2018 5:01 PM, Robin Murphy wrote: > On 02/02/18 05:40, Sricharan R wrote: >> Hi Robin/Vivek, >> >> On 2/1/2018 2:23 PM, Vivek Gautam wrote: >>> Hi, >>> >>> >>> On 1/31/2018 6:39 PM, Robin Murphy wrote: >>>> On 19/01/18 11:43, Vivek Gautam wrote: >>>>> From: Sricharan R <sricha...@codeaurora.org> >>>>> >>>>> 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. >>>> >>>> Don't we need to balance this with a device_link_del() in .remove_device >>>> (like exynos-iommu does)? >>> >>> Right. Will add device_link_del() call. Thanks for pointing out. >> >> The reason for not adding device_link_del from .remove_device was, the >> core device_del >> which calls the .remove_device from notifier, calls device_links_purge >> before that. >> That does the same thing as device_link_del. So by the time .remove_device >> is called, >> device_links for that device is already cleaned up. Vivek, you may want to >> check once that >> calling device_link_del from .remove_device has no effect, just to confirm >> once more. > > There is at least one path in which .remove_device is not called via the > notifier from device_del(), which is in the cleanup path of iommu_bus_init(). > AFAICS any links created by .add_device during that process would be left > dangling, because the device(s) would be live but otherwise disassociated > from the IOMMU afterwards. > > From a maintenance perspective it's easier to have the call in its logical > place even if it does nothing 99% of the time; that way we shouldn't have to > keep an eye out for subtle changes in the power management code or driver > core that might invalidate the device_del() reasoning above, and the power > management guys shouldn't have to comprehend the internals of the IOMMU API > to make sense of the unbalanced call if they ever want to change their API.
Ha, for a moment was thinking that with probe deferral add/remove_iommu_group in iommu_bus_init is dummy. But that may not be true for all Archs. Surely agree for the maintainability reason as well. Thanks. Regards, Sricharan -- "QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
