Re: [PATCH 1/2] iommu/mediatek-v1: Alloc building as module

[Robin Murphy]

^^Nit: presumably you meant "Allow" in the subject.

On 2021-03-25 17:16, Will Deacon wrote:

On Tue, Mar 23, 2021 at 01:58:00PM +0800, Yong Wu wrote:

This patch only adds support for building the IOMMU-v1 driver as module.
Correspondingly switch the config to tristate.

Signed-off-by: Yong Wu 
---
rebase on v5.12-rc2.
---
  drivers/iommu/Kconfig| 2 +-
  drivers/iommu/mtk_iommu_v1.c | 9 -
  2 files changed, 5 insertions(+), 6 deletions(-)


Both of these patches look fine to me, but you probably need to check
the setting of MODULE_OWNER after:

https://lore.kernel.org/r/f4de29d8330981301c1935e667b507254a2691ae.1616157612.git.robin.mur...@arm.com


Right, furthermore I would rather expect these patches on their own to 
hit the problem that my patch tries to avoid - where since mtk_iommu_ops 
is const, the current version of iommu_device_set_ops() is liable to 
blow up trying to write to rodata.


In fact I do wonder a little why that wasn't happening already - maybe 
the compiler is clever enough to tell that the assignment is redundant 
when THIS_MODULE == 0, and elides it :/


Robin.

Re: [PATCH 1/2] iommu/mediatek-v1: Alloc building as module

[Will Deacon]

On Tue, Mar 23, 2021 at 01:58:00PM +0800, Yong Wu wrote:
> This patch only adds support for building the IOMMU-v1 driver as module.
> Correspondingly switch the config to tristate.
> 
> Signed-off-by: Yong Wu 
> ---
> rebase on v5.12-rc2.
> ---
>  drivers/iommu/Kconfig| 2 +-
>  drivers/iommu/mtk_iommu_v1.c | 9 -
>  2 files changed, 5 insertions(+), 6 deletions(-)

Both of these patches look fine to me, but you probably need to check
the setting of MODULE_OWNER after:

https://lore.kernel.org/r/f4de29d8330981301c1935e667b507254a2691ae.1616157612.git.robin.mur...@arm.com

Will

[PATCH 1/2] iommu/mediatek-v1: Alloc building as module

[Yong Wu]

This patch only adds support for building the IOMMU-v1 driver as module.
Correspondingly switch the config to tristate.

Signed-off-by: Yong Wu 
---
rebase on v5.12-rc2.
---
 drivers/iommu/Kconfig| 2 +-
 drivers/iommu/mtk_iommu_v1.c | 9 -
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 192ef8f61310..bc93da48bed0 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -364,7 +364,7 @@ config MTK_IOMMU
  If unsure, say N here.
 
 config MTK_IOMMU_V1
-   bool "MTK IOMMU Version 1 (M4U gen1) Support"
+   tristate "MediaTek IOMMU Version 1 (M4U gen1) Support"
depends on ARM
depends on ARCH_MEDIATEK || COMPILE_TEST
select ARM_DMA_USE_IOMMU
diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
index 82ddfe9170d4..71370037083a 100644
--- a/drivers/iommu/mtk_iommu_v1.c
+++ b/drivers/iommu/mtk_iommu_v1.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -691,9 +692,7 @@ static struct platform_driver mtk_iommu_driver = {
.pm = _iommu_pm_ops,
}
 };
+module_platform_driver(mtk_iommu_driver);
 
-static int __init m4u_init(void)
-{
-   return platform_driver_register(_iommu_driver);
-}
-subsys_initcall(m4u_init);
+MODULE_DESCRIPTION("IOMMU API for MediaTek M4U v1 implementations");
+MODULE_LICENSE("GPL v2");
-- 
2.18.0