RE: [PATCH v2 3/4] iommu/exynos: Ensure that SYSMMU is added only once to its master device

["Sricharan" <sricharan@codeaurora.org>]

Hi,

>Hi Marek,
>
>On 09/01/17 12:03, Marek Szyprowski wrote:
>> This patch prepares Exynos IOMMU driver for deferred probing
>> support. Once it gets added, of_xlate() callback might be called
>> more than once for the same SYSMMU controller and master device
>> (for example it happens when masters device driver fails with
>> EPROBE_DEFER). This patch adds a check, which ensures that SYSMMU
>> controller is added to its master device (owner) only once.
>
>FWIW, I think it would be better to address that issue in the
>probe-deferral code itself, rather than fixing up individual IOMMU
>drivers. In fact, I *think* it's already covered in the latest version
>of the of_iommu_configure() changes[1], specifically this bit:
>
>	...
>	if (fwspec) {
>		if (fwspec->ops)
>			return fwspec->ops;
>	...
>
>Which should ensure that a successful IOMMU configuration happens
>exactly once between device creation and destruction.
>
>That said, this patch would still add robustness against actual error
>conditions, e.g. an erroneous DT which managed to specify the same
>phandle twice, although making it "if (WARN_ON(entry == data))" might be
>a good idea.
>

Yes, the latest version has the above mentioned fix of just returning back
from of_iommu_configure, if already configured. But anyways since this
patch is doing the check in xlate, which still can be called multiple times
even without deferring, incase of say multiple sids as well. But i
remember that sometime back Marek mentioned that the exynos
iommu currently always has "#iommu-cells = <0>" , but this patch might
be required if that changes as well.

Regards,
 Sricharan

>Robin.
>
>[1]:https://www.mail-archive.com/iommu@lists.linux-foundation.org/msg15333.html
>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>  drivers/iommu/exynos-iommu.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
>> index b0d537e6a445..8bf5a06a6c01 100644
>> --- a/drivers/iommu/exynos-iommu.c
>> +++ b/drivers/iommu/exynos-iommu.c
>> @@ -1253,7 +1253,7 @@ static int exynos_iommu_of_xlate(struct device *dev,
>>  {
>>  	struct exynos_iommu_owner *owner = dev->archdata.iommu;
>>  	struct platform_device *sysmmu = of_find_device_by_node(spec->np);
>> -	struct sysmmu_drvdata *data;
>> +	struct sysmmu_drvdata *data, *entry;
>>
>>  	if (!sysmmu)
>>  		return -ENODEV;
>> @@ -1272,6 +1272,10 @@ static int exynos_iommu_of_xlate(struct device *dev,
>>  		dev->archdata.iommu = owner;
>>  	}
>>
>> +	list_for_each_entry(entry, &owner->controllers, owner_node)
>> +		if (entry == data)
>> +			return 0;
>> +
>>  	list_add_tail(&data->owner_node, &owner->controllers);
>>  	data->master = dev;
>>
>>
>
>_______________________________________________
>iommu mailing list
>iommu@lists.linux-foundation.org
>https://lists.linuxfoundation.org/mailman/listinfo/iommu