2021-03-23 06:00:28

by Yong Wu (吴勇)

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

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

Signed-off-by: Yong Wu <[email protected]>
---
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 <linux/iommu.h>
#include <linux/iopoll.h>
#include <linux/list.h>
+#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/of_iommu.h>
#include <linux/of_irq.h>
@@ -691,9 +692,7 @@ static struct platform_driver mtk_iommu_driver = {
.pm = &mtk_iommu_pm_ops,
}
};
+module_platform_driver(mtk_iommu_driver);

-static int __init m4u_init(void)
-{
- return platform_driver_register(&mtk_iommu_driver);
-}
-subsys_initcall(m4u_init);
+MODULE_DESCRIPTION("IOMMU API for MediaTek M4U v1 implementations");
+MODULE_LICENSE("GPL v2");
--
2.18.0


2021-03-23 06:01:12

by Yong Wu (吴勇)

[permalink] [raw]
Subject: [PATCH 2/2] iommu/mediatek: Alloc building as module

The IOMMU in many SoC depends on the MM clocks and power-domain which
are device_initcall normally, thus the subsys_init here is not helpful.
This patch switches it to module_platform_driver which allow the
driver built as module.

Correspondingly switch the config to tristate.

Signed-off-by: Yong Wu <[email protected]>
---
drivers/iommu/Kconfig | 2 +-
drivers/iommu/mtk_iommu.c | 16 ++++------------
2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index bc93da48bed0..74f3e15edc14 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -349,7 +349,7 @@ config S390_AP_IOMMU
is not implemented as it is not necessary for VFIO.

config MTK_IOMMU
- bool "MTK IOMMU Support"
+ tristate "MediaTek IOMMU Support"
depends on ARCH_MEDIATEK || COMPILE_TEST
select ARM_DMA_USE_IOMMU
select IOMMU_API
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 6ecc007f07cd..a73ff3e20480 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -17,6 +17,7 @@
#include <linux/iopoll.h>
#include <linux/list.h>
#include <linux/mfd/syscon.h>
+#include <linux/module.h>
#include <linux/of_address.h>
#include <linux/of_iommu.h>
#include <linux/of_irq.h>
@@ -1079,16 +1080,7 @@ static struct platform_driver mtk_iommu_driver = {
.pm = &mtk_iommu_pm_ops,
}
};
+module_platform_driver(mtk_iommu_driver);

-static int __init mtk_iommu_init(void)
-{
- int ret;
-
- ret = platform_driver_register(&mtk_iommu_driver);
- if (ret != 0)
- pr_err("Failed to register MTK IOMMU driver\n");
-
- return ret;
-}
-
-subsys_initcall(mtk_iommu_init)
+MODULE_DESCRIPTION("IOMMU API for MediaTek M4U implementations");
+MODULE_LICENSE("GPL v2");
--
2.18.0

2021-03-25 17:18:25

by Will Deacon

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

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 <[email protected]>
> ---
> 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.murphy@arm.com

Will

2021-03-25 17:46:04

by Robin Murphy

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

^^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 <[email protected]>
>> ---
>> 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.murphy@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.