On Wed, Sep 29, 2021 at 4:46 PM Bjorn Andersson
[off-list ref] wrote:

On Wed 29 Sep 05:04 CDT 2021, Arnd Bergmann wrote:

quoted

On Wed, Sep 29, 2021 at 11:51 AM Will Deacon [off-list ref] wrote:

quoted

On Mon, Sep 27, 2021 at 05:22:13PM +0200, Arnd Bergmann wrote:

quoted

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 124c41adeca1..989c83acbfee 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig

@@ -308,7 +308,7 @@ config APPLE_DART
 config ARM_SMMU
      tristate "ARM Ltd. System MMU (SMMU) Support"
      depends on ARM64 || ARM || (COMPILE_TEST && !GENERIC_ATOMIC64)
-     depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't be =y
+     select QCOM_SCM
      select IOMMU_API
      select IOMMU_IO_PGTABLE_LPAE
      select ARM_DMA_USE_IOMMU if ARM

I don't want to get in the way of this patch because I'm also tired of the
randconfig failures caused by QCOM_SCM. However, ARM_SMMU is applicable to
a wide variety of (non-qcom) SoCs and so it seems a shame to require the
QCOM_SCM code to be included for all of those when it's not strictly needed
at all.

Good point, I agree that needs to be fixed. I think this additional
change should do the trick:

ARM_SMMU and QCOM_IOMMU are two separate implementations and both uses
QCOM_SCM. So both of them should select QCOM_SCM.

Right, I figured that out later as well.

"Unfortunately" the Qualcomm portion of ARM_SMMU is builtin
unconditionally, so going with something like select QCOM_SCM if
ARCH_QCOM would still require the stubs in qcom_scm.h.

Yes, sounds good. I also noticed that I still need one hack in there
if I do this:

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c

b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 55690af1b25d..36c304a8fc9b 100644

--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c

@@ -427,6 +427,9 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct

arm_smmu_device *smmu)
 {
        const struct device_node *np = smmu->dev->of_node;

+       if (!IS_ENABLED(CONFIG_QCOM_SCM))
+               return ERR_PTR(-ENXIO);
+
 #ifdef CONFIG_ACPI
        if (np == NULL) {
                /* Match platform for ACPI boot */


Otherwise it still breaks with ARM_SMMU=y and QCOM_SCM=m.

Splitting out the qualcomm portion of the arm_smmu driver using
a separate 'bool' symbol should also work, if  you prefer that
and can suggest a name and help text for that symbol. It would
look like

diff --git a/drivers/iommu/arm/arm-smmu/Makefile

b/drivers/iommu/arm/arm-smmu/Makefile
index e240a7bcf310..b0cc01aa20c9 100644

--- a/drivers/iommu/arm/arm-smmu/Makefile
+++ b/drivers/iommu/arm/arm-smmu/Makefile

@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_QCOM_IOMMU) += qcom_iommu.o
 obj-$(CONFIG_ARM_SMMU) += arm_smmu.o
-arm_smmu-objs += arm-smmu.o arm-smmu-impl.o arm-smmu-nvidia.o arm-smmu-qcom.o
+arm_smmu-objs += arm-smmu.o arm-smmu-impl.o arm-smmu-nvidia.o
+arm_smmu-$(CONFIG_ARM_SMMU_QCOM) += arm-smmu-qcom.o

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c

b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
index 9f465e146799..2c25cce38060 100644

--- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c

@@ -215,7 +215,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct

arm_smmu_device *smmu)
            of_device_is_compatible(np, "nvidia,tegra186-smmu"))
                return nvidia_smmu_impl_init(smmu);

-       smmu = qcom_smmu_impl_init(smmu);
+       if (IS_ENABLED(CONFIG_ARM_SMMU_QCOM))
+               smmu = qcom_smmu_impl_init(smmu);

        if (of_device_is_compatible(np, "marvell,ap806-smmu-500"))
                smmu->impl = &mrvl_mmu500_impl;



       Arnd