Hi Sam,
On 28.10.2022 21:12, Sam Protsenko wrote:
> Rework the driver so it can be built as a loadable module. That can be
> useful as not all ARM64 platforms need it. And that's ok for it to be a
> module because it's not a critical driver (platform can work when it's
> disabled).
>
> Also add the shutdown driver method, while at it. That was inspired by
> other IOMMU drivers, and can be useful e.g. for performing a kexec. See
> commit 1a4e90f25b2c ("iommu/rockchip: Perform a reset on shutdown") for
> example.
>
> Remove method and module exit function are not implemented, as the
> removal of IOMMUs cannot be done reliably. As Robin Murphy mentioned in
> [1]:
>
> ...it's better not to even pretend that removing an IOMMU's driver
> while other drivers are using it (usually via DMA ops without even
> realising) is going to have anything other than catastrophic
> results.
>
> [1] https://lore.kernel.org/lkml/20220702213724.3949-2-semen.protsenko@xxxxxxxxxx/T/#md7e1e3f5b2c9e7fa5bc28fe33e818b6aa4a7237c
>
> Signed-off-by: Sam Protsenko <semen.protsenko@xxxxxxxxxx>
MODULE_DEVICE_TABLE(of, sysmmu_of_match); is missing, so the driver
won't be automatically loaded, what breaks its operation if compiled as
module.
Also Exynos DRM and S5P-MFC drivers rely on the Exynos IOMMU being
built-in, so they need to be adjusted for modularized builds too imho,
at least in the Kconfig dependency.
> ---
> drivers/iommu/Kconfig | 2 +-
> drivers/iommu/exynos-iommu.c | 18 +++++++++++++++++-
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index dc5f7a156ff5..6f7055606679 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -259,7 +259,7 @@ config TEGRA_IOMMU_SMMU
> SoCs (Tegra30 up to Tegra210).
>
> config EXYNOS_IOMMU
> - bool "Exynos IOMMU Support"
> + tristate "Exynos IOMMU Support"
> depends on ARCH_EXYNOS || COMPILE_TEST
> depends on !CPU_BIG_ENDIAN # revisit driver if we can enable big-endian ptes
> select IOMMU_API
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 0d150b383d04..57492db877e2 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -16,6 +16,7 @@
> #include <linux/interrupt.h>
> #include <linux/kmemleak.h>
> #include <linux/list.h>
> +#include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> @@ -752,6 +753,16 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
> return ret;
> }
>
> +static void exynos_sysmmu_shutdown(struct platform_device *pdev)
> +{
> + struct sysmmu_drvdata *data = platform_get_drvdata(pdev);
> + struct device *dev = &pdev->dev;
> + int irq = platform_get_irq(pdev, 0);
> +
> + devm_free_irq(dev, irq, data);
> + pm_runtime_force_suspend(dev);
> +}
> +
> static int __maybe_unused exynos_sysmmu_suspend(struct device *dev)
> {
> struct sysmmu_drvdata *data = dev_get_drvdata(dev);
> @@ -799,8 +810,9 @@ static const struct of_device_id sysmmu_of_match[] = {
> { },
> };
>
> -static struct platform_driver exynos_sysmmu_driver __refdata = {
> +static struct platform_driver exynos_sysmmu_driver = {
> .probe = exynos_sysmmu_probe,
> + .shutdown = exynos_sysmmu_shutdown,
> .driver = {
> .name = "exynos-sysmmu",
> .of_match_table = sysmmu_of_match,
> @@ -1404,6 +1416,7 @@ static const struct iommu_ops exynos_iommu_ops = {
> .release_device = exynos_iommu_release_device,
> .pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE,
> .of_xlate = exynos_iommu_of_xlate,
> + .owner = THIS_MODULE,
> .default_domain_ops = &(const struct iommu_domain_ops) {
> .attach_dev = exynos_iommu_attach_device,
> .detach_dev = exynos_iommu_detach_device,
> @@ -1454,3 +1467,6 @@ static int __init exynos_iommu_init(void)
> return ret;
> }
> core_initcall(exynos_iommu_init);
> +
> +MODULE_DESCRIPTION("IOMMU driver for Exynos SoCs");
> +MODULE_LICENSE("GPL");
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
