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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Robin.

[1]:https://www.mail-archive.com/iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx/msg15333.html

> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> ---
>  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;
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  Powered by Linux