iommu/exynos: Skip unsupported devices instead of returning -ENODEV
Subsystems (3)
Marek Szyprowski
maintainerPATCH 1/1 iommu/exynos: Skip unsupported devices instead of returning -ENODEV
IOMMU core calls add_device() for every device on the given bus and since commit 19762d7095e6392b6ec56c363a6f29b2119488c2 ("iommu: Propagate error in add_iommu_group") all error from it are propagated to the iommu core. In case of platform bus there are many devices without sysmmu controller, so skip them gently instead of returning error, which would be propagated up to bus_set_iommu() and cause initialization failure.
---
Please queue this patch to v4.2-rcX line. It fixes regression introduced by a commit merged in parallel to the exynos iommu patches.
--- drivers/iommu/exynos-iommu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/iommu/exynos-iommu.c +2 -1
@@ -1119,8 +1119,9 @@ static int exynos_iommu_add_device(struct device *dev)struct iommu_group *group;int ret; +/* skip devices which doesn't have sysmmu controller */ if (!has_sysmmu (dev)) -return -ENODEV ; +return 0 ; group =iommu_group_get (dev);
Signature · 1 line
1.9.2
Joerg Roedel
maintainerPATCH 1/1 iommu: Ignore -ENODEV errors from add_device call-back returning -ENODEV
Hi Marek,
Review
Quoted 4 lines · Marek Szyprowski
(quoted diff) +2
-1+ /* skip devices which doesn't have sysmmu controller */ if (!has_sysmmu(dev)) - return -ENODEV; + return 0;
Thanks for reporting this! But I think that the -ENODEV return value could be of use for the iommu core in the future. Can you please try the attached patch, which just ignores -ENODEV as a return value from add_device?
Quoted 1 line
From
3c9e7507e93ff6c6e05e6ee2cb123b5d35d8c412Mon Sep 17 00:00:00 2001
From: Joerg Roedel <[email protected]>
Date: Mon, 29 Jun 2015 10:16:08 +0200
Subject: [PATCH] iommu: Ignore -ENODEV errors from add_device call-back
The -ENODEV error just means that the device is not translated by an IOMMU. We shouldn't bail out of iommu driver initialization when that happens, as this is a common scenario on ARM.
No returning -ENODEV in the drivers would be a bad idea, as the IOMMU core would have no indication whether a device is translated or not. This information is not used at the moment, but will probably be in the future.
--- drivers/iommu/iommu.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
drivers/iommu/iommu.c +12 -1
@@ -847,13 +847,24 @@ static int add_iommu_group(struct device *dev, void *data) {struct iommu_callback_data *cb = data;const struct iommu_ops *ops = cb->ops; +int ret;if (!ops->add_device)return 0 ;WARN_ON (dev->iommu_group); -return ops->add_device (dev); + ret = ops->add_device (dev); + +/* +* We ignore -ENODEV errors for now, as they just mean that the +* device is not translated by an IOMMU. We still care about +* other errors and fail to initialize when they happen. +*/ +if (ret == -ENODEV ) + ret =0 ; + +return ret; }static int remove_iommu_group (struct device *dev,void *data)
Signature · 1 line
1.8.4.5
A message that is not in the archive (sent off-list or not yet received): [email protected]
Marek Szyprowski
Tested-byRe: [PATCH] iommu: Ignore -ENODEV errors from add_device call-back returning -ENODEV
Hello,
Quoted 22 lines · Joerg Roedel
Hi Marek,
Review
Quoted 4 lines · Marek Szyprowski
(quoted diff) +2
-1+ /* skip devices which doesn't have sysmmu controller */ if (!has_sysmmu(dev)) - return -ENODEV; + return 0;Thanks for reporting this! But I think that the
-ENODEVreturn value could be of use for the iommu core in the future. Can you please try the attached patch, which just ignores-ENODEVas a return value fromadd_device?Quoted 1 line
From
3c9e7507e93ff6c6e05e6ee2cb123b5d35d8c412Mon Sep 17 00:00:00 2001From: Joerg Roedel <[email protected]>
Date: Mon, 29 Jun 2015 10:16:08 +0200
Subject: [PATCH] iommu: Ignore-ENODEVerrors fromadd_devicecall-backThe
-ENODEVerror just means that the device is not translated by an IOMMU. We shouldn't bail out of iommu driver initialization when that happens, as this is a common scenario on ARM.
Quoted 6 lines
No returning
-ENODEVin the drivers would be a bad idea, as the IOMMU core would have no indication whether a device is translated or not. This information is not used at the moment, but will probably be in the future.Signed-off-by Joerg Roedel <[email protected]>
Works fine!
Quoted 24 lines
--- drivers/iommu/iommu.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)drivers/iommu/iommu.c +5
-1@@ -847,13 +847,24 @@ static int add_iommu_group(struct device *dev, void *data) {struct iommu_callback_data *cb = data;const struct iommu_ops *ops = cb->ops; +int ret;if (!ops->add_device)return 0 ;WARN_ON (dev->iommu_group); -return ops->add_device (dev); + ret = ops->add_device (dev); + +/* +* We ignore -ENODEV errors for now, as they just mean that the
drivers/iommu/iommu.c static int add_iommu_group(struct device *dev, void *data) · line 870
Quoted 10 lines
drivers/iommu/iommu.c +7
-0+* device is not translated by an IOMMU. We still care about +* other errors and fail to initialize when they happen. +*/ +if (ret == -ENODEV ) + ret =0 ; + +return ret; }static int remove_iommu_group (struct device *dev,void *data)
Best regards
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland
Joerg Roedel
maintainerRe: [PATCH] iommu: Ignore -ENODEV errors from add_device call-back returning -ENODEV
Quoted 11 lines · Marek Szyprowski
Quoted 10 lines · Joerg Roedel
The
-ENODEVerror just means that the device is not translated by an IOMMU. We shouldn't bail out of iommu driver initialization when that happens, as this is a common scenario on ARM.No returning
-ENODEVin the drivers would be a bad idea, as the IOMMU core would have no indication whether a device is translated or not. This information is not used at the moment, but will probably be in the future.
Quoted 5 lines
Quoted 1 line
Signed-off-by Joerg Roedel <[email protected]>Works fine!
Tested-by Marek Szyprowski <[email protected]>
Thanks for testing! The patch is queued now.