On Wed, Sep 29, 2021 at 4:46 PM Bjorn Andersson bjorn.andersson@linaro.org wrote:
On Wed 29 Sep 05:04 CDT 2021, Arnd Bergmann wrote:
On Wed, Sep 29, 2021 at 11:51 AM Will Deacon will@kernel.org wrote:
On Mon, Sep 27, 2021 at 05:22:13PM +0200, Arnd Bergmann wrote:
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 ARMI 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