pinctrl: qcom: Kconfig: Rework PINCTRL_MSM to be a depenency rather then a selected config

Tweaks to allow pinctrl-msm code to be loadable as a module.

This is needed in order to support having the qcom-scm driver, which pinctrl-msm calls into, configured as a module.

This requires that we tweak Kconfigs selecting PINCTRL_MSM to also depend on QCOM_SCM || QCOM_SCM=n so that we match the module setting of QCOM_SCM.

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Andy Gross <[email protected]>

Cc Bjorn Andersson <[email protected]>

Cc Joerg Roedel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Jason Cooper <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Vinod Koul <[email protected]>

Cc Kalle Valo <[email protected]>

Cc Maulik Shah <[email protected]>

Cc Lina Iyer <[email protected]>

Cc Saravana Kannan <[email protected]>

Cc Todd Kjos <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Reviewed-by Bjorn Andersson <[email protected]>

Signed-off-by John Stultz <[email protected]>

Allow the qcom_scm driver to be loadable as a permenent module.

This still uses the "depends on QCOM_SCM || !QCOM_SCM" bit to ensure that drivers that call into the qcom_scm driver are also built as modules. While not ideal in some cases its the only safe way I can find to avoid build errors without having those drivers select QCOM_SCM and have to force it on (as QCOM_SCM=n can be valid for those drivers).

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Andy Gross <[email protected]>

Cc Bjorn Andersson <[email protected]>

Cc Joerg Roedel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Jason Cooper <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Vinod Koul <[email protected]>

Cc Kalle Valo <[email protected]>

Cc Maulik Shah <[email protected]>

Cc Lina Iyer <[email protected]>

Cc Saravana Kannan <[email protected]>

Cc Todd Kjos <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Acked-by Kalle Valo <[email protected]>

Acked-by Greg Kroah-Hartman <[email protected]>

Reviewed-by Bjorn Andersson <[email protected]>

Signed-off-by John Stultz <[email protected]>

Quoted 2 lines · John Stultz

Allow the qcom_scm driver to be loadable as a permenent module.

I applied this patch to the pinctrl tree as well, I suppose that was the intention. If someone gets upset I can always pull it out.

Quoted 4 lines · Linus Walleij

Quoted 2 lines · John Stultz

Allow the qcom_scm driver to be loadable as a permenent module.

...

Quoted 3 lines

I applied this patch to the pinctrl tree as well, I suppose that was the intention. If someone gets upset I can always pull it out.

Will: You ok with this?

Quoted 5 lines · John Stultz

Quoted 4 lines · Linus Walleij

Quoted 2 lines · John Stultz

Allow the qcom_scm driver to be loadable as a permenent module.

Quoted 6 lines

...

Quoted 3 lines

I applied this patch to the pinctrl tree as well, I suppose that was the intention. If someone gets upset I can always pull it out.

Will: You ok with this?

We didn't come up with something better, so I can live with it.

Not sure
about the otehr issues that were reported by Robin though -- your RFC for fixing those looked a bit more controversial ;)

Allow the qcom_scm driver to be loadable as a permenent module.

This still uses the "depends on QCOM_SCM || !QCOM_SCM" bit to ensure that drivers that call into the qcom_scm driver are also built as modules. While not ideal in some cases its the only safe way I can find to avoid build errors without having those drivers select QCOM_SCM and have to force it on (as QCOM_SCM=n can be valid for those drivers).

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Andy Gross <[email protected]>

Cc Bjorn Andersson <[email protected]>

Cc Joerg Roedel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Jason Cooper <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Vinod Koul <[email protected]>

Cc Kalle Valo <[email protected]>

Cc Maulik Shah <[email protected]>

Cc Lina Iyer <[email protected]>

Cc Saravana Kannan <[email protected]>

Cc Todd Kjos <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Acked-by Kalle Valo <[email protected]>

Acked-by Greg Kroah-Hartman <[email protected]>

Reviewed-by Bjorn Andersson <[email protected]>

Signed-off-by John Stultz <[email protected]>

---
v3:
* Fix __arm_smccc_smc build issue reported by kernel test robot <[email protected]>
v4:
* Add "depends on QCOM_SCM || !QCOM_SCM" bit to ath10k config that requires it.
v5:
* Fix QCOM_QCM typo in Kconfig, it should be QCOM_SCM

---
 drivers/firmware/Kconfig                | 4 ++--
 drivers/firmware/Makefile               | 3 ++-
 drivers/firmware/qcom_scm.c             | 4 ++++
 drivers/iommu/Kconfig                   | 2 ++
 drivers/net/wireless/ath/ath10k/Kconfig | 1 +
 5 files changed, 11 insertions(+), 3 deletions(-)

drivers/firmware/Kconfig +2 -2

index 3315e3c215864..5e369928bc567 100644
@@ -235,8 +235,8 @@ config INTEL_STRATIX10_RSU
 	  Say Y here if you want Intel RSU support.
 
 config QCOM_SCM
-	bool
-	depends on ARM || ARM64
+	tristate "Qcom SCM driver"
+	depends on (ARM && HAVE_ARM_SMCCC) || ARM64
 	select RESET_CONTROLLER
 
 config QCOM_SCM_DOWNLOAD_MODE_DEFAULT

drivers/firmware/Makefile +2 -1

index 5e013b6a3692e..523173cbff335 100644
@@ -17,7 +17,8 @@ obj-$(CONFIG_ISCSI_IBFT)	+= iscsi_ibft.o
 obj-$(CONFIG_FIRMWARE_MEMMAP)	+= memmap.o
 obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
 obj-$(CONFIG_FW_CFG_SYSFS)	+= qemu_fw_cfg.o
-obj-$(CONFIG_QCOM_SCM)		+= qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
+obj-$(CONFIG_QCOM_SCM)		+= qcom-scm.o
+qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
 obj-$(CONFIG_TI_SCI_PROTOCOL)	+= ti_sci.o
 obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o
 obj-$(CONFIG_TURRIS_MOX_RWTM)	+= turris-mox-rwtm.o

drivers/firmware/qcom_scm.c +4 -0

index 7be48c1bec96d..6f431b73e617d 100644
@@ -1280,6 +1280,7 @@ static const struct of_device_id qcom_scm_dt_match[] = {
 	{ .compatible = "qcom,scm" },
 	{}
 };
+MODULE_DEVICE_TABLE(of, qcom_scm_dt_match);
 
 static struct platform_driver qcom_scm_driver = {
 	.driver = {

@@ -1295,3 +1296,6 @@ static int __init qcom_scm_init(void)
 	return platform_driver_register(&qcom_scm_driver);
 }
 subsys_initcall(qcom_scm_init);
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. SCM driver");
+MODULE_LICENSE("GPL v2");

drivers/iommu/Kconfig +0 -0

index 04878caf6da49..c64d7a2b65134 100644

Quoted 24 lines · John Stultz

drivers/firmware/qcom_scm.c +4 -0

index 7be48c1bec96d..6f431b73e617d 100644
@@ -1280,6 +1280,7 @@ static const struct of_device_id qcom_scm_dt_match[] = {
 	{ .compatible = "qcom,scm" },
 	{}
 };
+MODULE_DEVICE_TABLE(of, qcom_scm_dt_match);
 
 static struct platform_driver qcom_scm_driver = {
 	.driver = {

@@ -1295,3 +1296,6 @@ static int __init qcom_scm_init(void)
 	return platform_driver_register(&qcom_scm_driver);
 }
 subsys_initcall(qcom_scm_init);
+
+MODULE_DESCRIPTION("Qualcomm Technologies, Inc. SCM driver");
+MODULE_LICENSE("GPL v2");

drivers/iommu/Kconfig +0 -0

index 04878caf6da49..c64d7a2b65134 100644

drivers/iommu/Kconfig config SPAPR_TCE_IOMMU · line 254

Quoted 7 lines

drivers/iommu/Kconfig +1 -0

@@ -248,6 +248,7 @@ config SPAPR_TCE_IOMMU
 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 IOMMU_API
 	select IOMMU_IO_PGTABLE_LPAE
 	select ARM_DMA_USE_IOMMU if ARM

This, in conjunction with deferred probe timeout, causes mayhem on Tegra186. The problem, as far as I can tell, is that there are various devices that are hooked up to the ARM SMMU, but if ARM SMMU ends up being built as a loadable module, then those devices will initialize without IOMMU support (because deferred probe will timeout before the ARM SMMU module can be loaded from the root filesystem).

Unfortunately, the ARM SMMU module will eventually end up being loaded once the root filesystem has been mounted (for example via SDHCI or Ethernet, both with using just plain, non-IOMMU-backed DMA API) and then initialize, configuring as "fault by default", which then results from a slew of SMMU faults from all the devices that have previously configured themselves without IOMMU support.

Quoted 7 lines · Thierry Reding

On Fri, Nov 06, 2020 at 04:27:10AM +0000, John Stultz wrote:
Unfortunately, the ARM SMMU module will eventually end up being loaded once the root filesystem has been mounted (for example via SDHCI or Ethernet, both with using just plain, non-IOMMU-backed DMA API) and then initialize, configuring as "fault by default", which then results from a slew of SMMU faults from all the devices that have previously configured themselves without IOMMU support.

I wonder if fw_devlink=on would help here?

But either way, I'd be more inclined to revert this change if it's causing problems for !QCOM devices.

Linus -- please can you drop this one (patch 3/3) for now, given that it's causing problems?

Quoted 9 lines · Will Deacon

Quoted 7 lines · Thierry Reding

On Fri, Nov 06, 2020 at 04:27:10AM +0000, John Stultz wrote:
Unfortunately, the ARM SMMU module will eventually end up being loaded once the root filesystem has been mounted (for example via SDHCI or Ethernet, both with using just plain, non-IOMMU-backed DMA API) and then initialize, configuring as "fault by default", which then results from a slew of SMMU faults from all the devices that have previously configured themselves without IOMMU support.

Quoted 7 lines

I wonder if fw_devlink=on would help here?

But either way, I'd be more inclined to revert this change if it's causing problems for !QCOM devices.

Linus -- please can you drop this one (patch 3/3) for now, given that it's causing problems?

Agreed. Apologies again for the trouble.

I do feel like the probe timeout to handle optional links is causing a lot of the trouble here. I expect fw_devlink would solve this, but it may be awhile before it can be always enabled. I may see about pushing the default probe timeout value to be a little further out than init (I backed away from my last attempt as I didn't want to cause long (30 second) delays for cases like NFS root, but maybe 2-5 seconds would be enough to make things work better for everyone).

Quoted 20 lines · John Stultz

Quoted 9 lines · Will Deacon

Quoted 7 lines · Thierry Reding

On Fri, Nov 06, 2020 at 04:27:10AM +0000, John Stultz wrote:
Unfortunately, the ARM SMMU module will eventually end up being loaded once the root filesystem has been mounted (for example via SDHCI or Ethernet, both with using just plain, non-IOMMU-backed DMA API) and then initialize, configuring as "fault by default", which then results from a slew of SMMU faults from all the devices that have previously configured themselves without IOMMU support.

Quoted 7 lines

I wonder if fw_devlink=on would help here?

But either way, I'd be more inclined to revert this change if it's causing problems for !QCOM devices.

Linus -- please can you drop this one (patch 3/3) for now, given that it's causing problems?

Agreed. Apologies again for the trouble.

Quoted 7 lines

I do feel like the probe timeout to handle optional links is causing a lot of the trouble here. I expect fw_devlink would solve this, but it may be awhile before it can be always enabled. I may see about pushing the default probe timeout value to be a little further out than init (I backed away from my last attempt as I didn't want to cause long (30 second) delays for cases like NFS root, but maybe 2-5 seconds would be enough to make things work better for everyone).

I think there are two problems here: 1) the deferred probe timeout can cause a mismatch between what SMMU masters and the SMMU think is going on and 2) a logistical problem of dealing with the SMMU driver being a loadable module.

The second problem can be dealt with by shipping the module in the initial ramdisk. That's a bit annoying, but perhaps the right thing to do. At least on Tegra we need this because all the devices that carry the root filesystem (Ethernet for NFS and SDHCI/USB/SATA/PCI for disk boot) are SMMU masters and will start to fault once the SMMU driver is loaded.

Linus -- please can you drop this one (patch 3/3) for now, given that it's causing problems?

Quoted 2 lines · Will Deacon

Linus -- please can you drop this one (patch 3/3) for now, given that it's causing problems?

Reverted now, sorry for missing to do this earlier.

drivers/iommu/Kconfig config SPAPR_TCE_IOMMU · line 254

Quoted 12 lines · John Stultz

drivers/iommu/Kconfig +1 -0

index 04878caf6da49..c64d7a2b65134 100644
@@ -248,6 +248,7 @@ config SPAPR_TCE_IOMMU
 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 IOMMU_API
      select IOMMU_IO_PGTABLE_LPAE
      select ARM_DMA_USE_IOMMU if ARM

This, in conjunction with deferred probe timeout, causes mayhem on Tegra186. The problem, as far as I can tell, is that there are various devices that are hooked up to the ARM SMMU, but if ARM SMMU ends up being built as a loadable module, then those devices will initialize without IOMMU support (because deferred probe will timeout before the ARM SMMU module can be loaded from the root filesystem).

Unfortunately, the ARM SMMU module will eventually end up being loaded once the root filesystem has been mounted (for example via SDHCI or Ethernet, both with using just plain, non-IOMMU-backed DMA API) and then initialize, configuring as "fault by default", which then results from a slew of SMMU faults from all the devices that have previously configured themselves without IOMMU support.

This patch reworks PINCTRL_MSM to be a visible option, and instead of having the various SoC specific drivers select PINCTRL_MSM, this switches those configs to depend on PINCTRL_MSM.

This is useful, as it will be needed in order to cleanly support having the qcom-scm driver, which pinctrl-msm calls into, configured as a module. Without this change, we would eventually have to add dependency lines to every config that selects PINCTRL_MSM, and that would becomes a maintenance headache.

We also add PINCTRL_MSM to the arm64 defconfig to avoid surprises as otherwise PINCTRL_MSM/IPQ* options previously enabled, will be off.

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Andy Gross <[email protected]>

Cc Bjorn Andersson <[email protected]>

Cc Joerg Roedel <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Jason Cooper <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Vinod Koul <[email protected]>

Cc Kalle Valo <[email protected]>

Cc Maulik Shah <[email protected]>

Cc Lina Iyer <[email protected]>

Cc Saravana Kannan <[email protected]>

Cc Todd Kjos <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Reviewed-by Bjorn Andersson <[email protected]>

Signed-off-by John Stultz <[email protected]>