Re: [PATCH 2/2] iommu: Streamline registration interface

[Will Deacon]

On Fri, Mar 19, 2021 at 12:52:02PM +, Robin Murphy wrote:
> Rather than have separate opaque setter functions that are easy to
> overlook and lead to repetitive boilerplate in drivers, let's pass the
> relevant initialisation parameters directly to iommu_device_register().
> 
> Signed-off-by: Robin Murphy 
> ---
>  drivers/iommu/amd/init.c|  3 +--
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  5 +---
>  drivers/iommu/arm/arm-smmu/arm-smmu.c   |  5 +---
>  drivers/iommu/arm/arm-smmu/qcom_iommu.c |  5 +---
>  drivers/iommu/exynos-iommu.c|  5 +---
>  drivers/iommu/fsl_pamu_domain.c |  4 +--
>  drivers/iommu/intel/dmar.c  |  4 +--
>  drivers/iommu/intel/iommu.c |  3 +--
>  drivers/iommu/iommu.c   |  7 -
>  drivers/iommu/ipmmu-vmsa.c  |  6 +
>  drivers/iommu/msm_iommu.c   |  5 +---
>  drivers/iommu/mtk_iommu.c   |  5 +---
>  drivers/iommu/mtk_iommu_v1.c|  4 +--
>  drivers/iommu/omap-iommu.c  |  5 +---
>  drivers/iommu/rockchip-iommu.c  |  5 +---
>  drivers/iommu/s390-iommu.c  |  4 +--
>  drivers/iommu/sprd-iommu.c  |  5 +---
>  drivers/iommu/sun50i-iommu.c|  5 +---
>  drivers/iommu/tegra-gart.c  |  5 +---
>  drivers/iommu/tegra-smmu.c  |  5 +---
>  drivers/iommu/virtio-iommu.c|  5 +---
>  include/linux/iommu.h   | 29 -
>  22 files changed, 31 insertions(+), 98 deletions(-)

I was worried this might blow up with !CONFIG_IOMMU_API, but actually
it all looks fine and is much cleaner imo so:

Acked-by: Will Deacon 

Will

Re: [PATCH 2/2] iommu: Streamline registration interface

[Christoph Hellwig]

> +int iommu_device_register(struct iommu_device *iommu, const struct iommu_ops 
> *ops,

It would be nice to avoid the pointlessly overlong line here.

> + struct device *hwdev)
>  {
> + iommu->ops = ops;
> + if (hwdev)
> + iommu->fwnode = hwdev->fwnode;

This function could use a kerneldoc comment now.  Especially the hwdev
agument isn't exactly obvious.

[PATCH 2/2] iommu: Streamline registration interface

[Robin Murphy]

Rather than have separate opaque setter functions that are easy to
overlook and lead to repetitive boilerplate in drivers, let's pass the
relevant initialisation parameters directly to iommu_device_register().

Signed-off-by: Robin Murphy 
---
 drivers/iommu/amd/init.c|  3 +--
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  5 +---
 drivers/iommu/arm/arm-smmu/arm-smmu.c   |  5 +---
 drivers/iommu/arm/arm-smmu/qcom_iommu.c |  5 +---
 drivers/iommu/exynos-iommu.c|  5 +---
 drivers/iommu/fsl_pamu_domain.c |  4 +--
 drivers/iommu/intel/dmar.c  |  4 +--
 drivers/iommu/intel/iommu.c |  3 +--
 drivers/iommu/iommu.c   |  7 -
 drivers/iommu/ipmmu-vmsa.c  |  6 +
 drivers/iommu/msm_iommu.c   |  5 +---
 drivers/iommu/mtk_iommu.c   |  5 +---
 drivers/iommu/mtk_iommu_v1.c|  4 +--
 drivers/iommu/omap-iommu.c  |  5 +---
 drivers/iommu/rockchip-iommu.c  |  5 +---
 drivers/iommu/s390-iommu.c  |  4 +--
 drivers/iommu/sprd-iommu.c  |  5 +---
 drivers/iommu/sun50i-iommu.c|  5 +---
 drivers/iommu/tegra-gart.c  |  5 +---
 drivers/iommu/tegra-smmu.c  |  5 +---
 drivers/iommu/virtio-iommu.c|  5 +---
 include/linux/iommu.h   | 29 -
 22 files changed, 31 insertions(+), 98 deletions(-)

diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 321f5906e6ed..e1ef922d9f8f 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -1935,8 +1935,7 @@ static int __init iommu_init_pci(struct amd_iommu *iommu)
 
iommu_device_sysfs_add(>iommu, >dev->dev,
   amd_iommu_groups, "ivhd%d", iommu->index);
-   iommu_device_set_ops(>iommu, _iommu_ops);
-   iommu_device_register(>iommu);
+   iommu_device_register(>iommu, _iommu_ops, NULL);
 
return pci_enable_device(iommu->dev);
 }
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index b82000519af6..ecc6cfe3ae90 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3621,10 +3621,7 @@ static int arm_smmu_device_probe(struct platform_device 
*pdev)
if (ret)
return ret;
 
-   iommu_device_set_ops(>iommu, _smmu_ops);
-   iommu_device_set_fwnode(>iommu, dev->fwnode);
-
-   ret = iommu_device_register(>iommu);
+   ret = iommu_device_register(>iommu, _smmu_ops, dev);
if (ret) {
dev_err(dev, "Failed to register iommu\n");
return ret;
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c 
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 11ca963c4b93..0a697cb0d2f8 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -,10 +,7 @@ static int arm_smmu_device_probe(struct platform_device 
*pdev)
return err;
}
 
-   iommu_device_set_ops(>iommu, _smmu_ops);
-   iommu_device_set_fwnode(>iommu, dev->fwnode);
-
-   err = iommu_device_register(>iommu);
+   err = iommu_device_register(>iommu, _smmu_ops, dev);
if (err) {
dev_err(dev, "Failed to register iommu\n");
return err;
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c 
b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 7f280c8d5c53..4294abe389b2 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -847,10 +847,7 @@ static int qcom_iommu_device_probe(struct platform_device 
*pdev)
return ret;
}
 
-   iommu_device_set_ops(_iommu->iommu, _iommu_ops);
-   iommu_device_set_fwnode(_iommu->iommu, dev->fwnode);
-
-   ret = iommu_device_register(_iommu->iommu);
+   ret = iommu_device_register(_iommu->iommu, _iommu_ops, dev);
if (ret) {
dev_err(dev, "Failed to register iommu\n");
return ret;
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index de324b4eedfe..f887c3e111c1 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -630,10 +630,7 @@ static int exynos_sysmmu_probe(struct platform_device 
*pdev)
if (ret)
return ret;
 
-   iommu_device_set_ops(>iommu, _iommu_ops);
-   iommu_device_set_fwnode(>iommu, >of_node->fwnode);
-
-   ret = iommu_device_register(>iommu);
+   ret = iommu_device_register(>iommu, _iommu_ops, dev);
if (ret)
return ret;
 
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index b2110767caf4..1a15bd5da358 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -1053,9 +1053,7 @@ int __init pamu_domain_init(void)
if