| Message ID | 20210705065657.30356-1-m.szyprowski@samsung.com |
|---|---|
| State | Accepted |
| Commit | ce36c94214f0d534aad4adb4219cc046270948fb |
| Headers | show |
| Series | iommu: qcom: Revert "iommu/arm: Cleanup resources in case of probe error path" | expand |
On 21/07/05 08:56AM, Marek Szyprowski wrote: > QCOM IOMMU driver calls bus_set_iommu() for every IOMMU device controller, > what fails for the second and latter IOMMU devices. This is intended and > must be not fatal to the driver registration process. Also the cleanup > path should take care of the runtime PM state, what is missing in the > current patch. Revert relevant changes to the QCOM IOMMU driver until > a proper fix is prepared. > Apologies for the broken patch I don't have any arm machine to test the patches. Is this bug unique to qcom iommu? [...] Thanks, Amey
On 05.07.2021 16:30, Amey Narkhede wrote: > On 21/07/05 08:56AM, Marek Szyprowski wrote: >> QCOM IOMMU driver calls bus_set_iommu() for every IOMMU device controller, >> what fails for the second and latter IOMMU devices. This is intended and >> must be not fatal to the driver registration process. Also the cleanup >> path should take care of the runtime PM state, what is missing in the >> current patch. Revert relevant changes to the QCOM IOMMU driver until >> a proper fix is prepared. >> > Apologies for the broken patch I don't have any arm machine to test the > patches. Is this bug unique to qcom iommu? Frankly, I have no idea. Just grep for bus_set_iommu() and check if the caller might be executed more than once. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
On Mon, Jul 05, 2021 at 08:56:57AM +0200, Marek Szyprowski wrote: > QCOM IOMMU driver calls bus_set_iommu() for every IOMMU device controller, > what fails for the second and latter IOMMU devices. This is intended and > must be not fatal to the driver registration process. Also the cleanup > path should take care of the runtime PM state, what is missing in the > current patch. Revert relevant changes to the QCOM IOMMU driver until > a proper fix is prepared. > > This partially reverts commit 249c9dc6aa0db74a0f7908efd04acf774e19b155. > > Fixes: 249c9dc6aa0d ("iommu/arm: Cleanup resources in case of probe error path") > Suggested-by: Will Deacon <will@kernel.org> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/iommu/arm/arm-smmu/qcom_iommu.c | 13 ++----------- > 1 file changed, 2 insertions(+), 11 deletions(-) Thanks, Marek: Acked-by: Will Deacon <will@kernel.org> Joerg -- please can you pick this up as a fix? Cheers, Will > diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c > index 25ed444ff94d..021cf8f65ffc 100644 > --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c > +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c > @@ -849,12 +849,10 @@ static int qcom_iommu_device_probe(struct platform_device *pdev) > ret = iommu_device_register(&qcom_iommu->iommu, &qcom_iommu_ops, dev); > if (ret) { > dev_err(dev, "Failed to register iommu\n"); > - goto err_sysfs_remove; > + return ret; > } > > - ret = bus_set_iommu(&platform_bus_type, &qcom_iommu_ops); > - if (ret) > - goto err_unregister_device; > + bus_set_iommu(&platform_bus_type, &qcom_iommu_ops); > > if (qcom_iommu->local_base) { > pm_runtime_get_sync(dev); > @@ -863,13 +861,6 @@ static int qcom_iommu_device_probe(struct platform_device *pdev) > } > > return 0; > - > -err_unregister_device: > - iommu_device_unregister(&qcom_iommu->iommu); > - > -err_sysfs_remove: > - iommu_device_sysfs_remove(&qcom_iommu->iommu); > - return ret; > } > > static int qcom_iommu_device_remove(struct platform_device *pdev) > -- > 2.17.1 >
On Mon, Jul 05, 2021 at 08:56:57AM +0200, Marek Szyprowski wrote: > QCOM IOMMU driver calls bus_set_iommu() for every IOMMU device controller, > what fails for the second and latter IOMMU devices. This is intended and > must be not fatal to the driver registration process. Also the cleanup > path should take care of the runtime PM state, what is missing in the > current patch. Revert relevant changes to the QCOM IOMMU driver until > a proper fix is prepared. > > This partially reverts commit 249c9dc6aa0db74a0f7908efd04acf774e19b155. > > Fixes: 249c9dc6aa0d ("iommu/arm: Cleanup resources in case of probe error path") > Suggested-by: Will Deacon <will@kernel.org> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/iommu/arm/arm-smmu/qcom_iommu.c | 13 ++----------- > 1 file changed, 2 insertions(+), 11 deletions(-) Applied to iommu/fixes, thanks.
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c b/drivers/iommu/arm/arm-smmu/qcom_iommu.c index 25ed444ff94d..021cf8f65ffc 100644 --- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c +++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c @@ -849,12 +849,10 @@ static int qcom_iommu_device_probe(struct platform_device *pdev) ret = iommu_device_register(&qcom_iommu->iommu, &qcom_iommu_ops, dev); if (ret) { dev_err(dev, "Failed to register iommu\n"); - goto err_sysfs_remove; + return ret; } - ret = bus_set_iommu(&platform_bus_type, &qcom_iommu_ops); - if (ret) - goto err_unregister_device; + bus_set_iommu(&platform_bus_type, &qcom_iommu_ops); if (qcom_iommu->local_base) { pm_runtime_get_sync(dev); @@ -863,13 +861,6 @@ static int qcom_iommu_device_probe(struct platform_device *pdev) } return 0; - -err_unregister_device: - iommu_device_unregister(&qcom_iommu->iommu); - -err_sysfs_remove: - iommu_device_sysfs_remove(&qcom_iommu->iommu); - return ret; } static int qcom_iommu_device_remove(struct platform_device *pdev)
QCOM IOMMU driver calls bus_set_iommu() for every IOMMU device controller, what fails for the second and latter IOMMU devices. This is intended and must be not fatal to the driver registration process. Also the cleanup path should take care of the runtime PM state, what is missing in the current patch. Revert relevant changes to the QCOM IOMMU driver until a proper fix is prepared. This partially reverts commit 249c9dc6aa0db74a0f7908efd04acf774e19b155. Fixes: 249c9dc6aa0d ("iommu/arm: Cleanup resources in case of probe error path") Suggested-by: Will Deacon <will@kernel.org> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/iommu/arm/arm-smmu/qcom_iommu.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) -- 2.17.1