On Fri, 4 Nov 2022 at 13:10, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote: > > On 03.11.2022 20:51, Sam Protsenko wrote: > > As exynos-iommu driver is not a critical platform driver, it can be > > converted to a loadable module to avoid loading it on non-Exynos > > platforms in order to improve the RAM footprint. This patch series > > converts it to a module and does some related cleanups. IOMMU/DMA > > specifics were taken into the account, so remove/exit methods weren't > > added. > > > > There are two drivers using CONFIG_EXYNOS_IOMMU in their code: > > DRM_EXYNOS and S5P_MFC. Both were checked, and only a slight change was > > needed for S5P_MFC driver (patch #6). > > Funny, compiling this driver as a module revealed an issue in the driver > initialization sequence, here is a fix that need to be applied before > this patchset: > > https://lore.kernel.org/all/20221104115511.28256-1-m.szyprowski@xxxxxxxxxxx/ > I wonder why I didn't stumble upon that issue. Anyway, the fix looks valid. I'll wait until you finish looking into that NULL pointer dereference, and then we can figure out how to organize our patches. Maybe I could send v3, including your patches in the beginning of the series, etc. > Besides that, the driver nukes with NULL pointer dereference in > exynos_iommu_of_xlate() when compiled as a module on ARM 64bit > Exynos5433 based TM2e board. ARM 32bit based board works fine. I'm > checking this issue now. > Thanks for looking into that, Marek! I've tested the modularization patch series using my emulated translation driver [1,2]. It works fine, but exynos_iommu_of_xlate() is only executed once in my case, when the test driver module is loaded. It's finished successfully though. Not sure what is the cause of NULL pointer dereferences you're seeing, alas I still don't have real users of SysMMU driver on my platform, and my test driver is the only way for me to test it. Please let me know if I can help in any way (e.g. I can trace some calls on my board for you). [1] https://github.com/joe-skb7/linux/blob/e850-96-mainline-iommu/drivers/iommu/samsung-iommu-test.c#L263 [2] https://github.com/joe-skb7/linux/blob/e850-96-mainline-iommu/arch/arm64/boot/dts/exynos/exynos850.dtsi#L477 > > Changes in v2: > > - Extracted the "shutdown" method addition into a separate patch > > - Added MODULE_DEVICE_TABLE(of, ...) to support hot-plug loading > > - Added MODULE_ALIAS("platform:exynos-sysmmu") > > - Added fix for S5P_MFC driver to work correctly with EXYNOS_IOMMU=m > > - Fixed checkpatch coding style suggestion with "--strict" flag > > - Rebased on top of most recent joro/iommu.git:next > > > > Sam Protsenko (6): > > iommu: Export iommu_group_default_domain() API > > iommu/exynos: Fix retval on getting clocks in probe > > iommu/exynos: Modularize the driver > > iommu/exynos: Implement shutdown driver method > > iommu/exynos: Rearrange the platform driver code > > media: platform: Use IS_ENABLED() to check EXYNOS_IOMMU in s5p_mfc > > > > drivers/iommu/Kconfig | 2 +- > > drivers/iommu/exynos-iommu.c | 355 +++++++++--------- > > drivers/iommu/iommu.c | 1 + > > .../platform/samsung/s5p-mfc/s5p_mfc_iommu.h | 4 +- > > 4 files changed, 191 insertions(+), 171 deletions(-) > > > Best regards > -- > Marek Szyprowski, PhD > Samsung R&D Institute Poland >
