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
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Reply via email to