Re: [PATCH 2/2] [v2] qcom_scm: hide Kconfig symbol

2021-09-29 Thread Bjorn Andersson
On Tue 28 Sep 02:50 CDT 2021, Arnd Bergmann wrote:

> From: Arnd Bergmann 
> 
> Now that SCM can be a loadable module, we have to add another
> dependency to avoid link failures when ipa or adreno-gpu are
> built-in:
> 
> aarch64-linux-ld: drivers/net/ipa/ipa_main.o: in function `ipa_probe':
> ipa_main.c:(.text+0xfc4): undefined reference to `qcom_scm_is_available'
> 
> ld.lld: error: undefined symbol: qcom_scm_is_available
> >>> referenced by adreno_gpu.c
> >>>   gpu/drm/msm/adreno/adreno_gpu.o:(adreno_zap_shader_load) in 
> >>> archive drivers/built-in.a
> 
> This can happen when CONFIG_ARCH_QCOM is disabled and we don't select
> QCOM_MDT_LOADER, but some other module selects QCOM_SCM. Ideally we'd
> use a similar dependency here to what we have for QCOM_RPROC_COMMON,
> but that causes dependency loops from other things selecting QCOM_SCM.
> 
> This appears to be an endless problem, so try something different this
> time:
> 
>  - CONFIG_QCOM_SCM becomes a hidden symbol that nothing 'depends on'
>but that is simply selected by all of its users
> 
>  - All the stubs in include/linux/qcom_scm.h can go away
> 
>  - arm-smccc.h needs to provide a stub for __arm_smccc_smc() to
>allow compile-testing QCOM_SCM on all architectures.
> 
>  - To avoid a circular dependency chain involving RESET_CONTROLLER
>and PINCTRL_SUNXI, drop the 'select RESET_CONTROLLER' statement.
>According to my testing this still builds fine, and the QCOM
>platform selects this symbol already.
> 
> Acked-by: Kalle Valo 
> Signed-off-by: Arnd Bergmann 
> ---
> Changes in v2:
>   - drop the 'select RESET_CONTROLLER' line, rather than adding
> more of the same
> ---
>  drivers/firmware/Kconfig|  5 +-
>  drivers/gpu/drm/msm/Kconfig |  4 +-
>  drivers/iommu/Kconfig   |  2 +-
>  drivers/media/platform/Kconfig  |  2 +-
>  drivers/mmc/host/Kconfig|  2 +-
>  drivers/net/ipa/Kconfig |  1 +
>  drivers/net/wireless/ath/ath10k/Kconfig |  2 +-
>  drivers/pinctrl/qcom/Kconfig|  3 +-
>  include/linux/arm-smccc.h   | 10 
>  include/linux/qcom_scm.h| 71 -
>  10 files changed, 20 insertions(+), 82 deletions(-)
> 
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index 220a58cf0a44..cda7d7162cbb 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -203,10 +203,7 @@ config INTEL_STRATIX10_RSU
> Say Y here if you want Intel RSU support.
>  
>  config QCOM_SCM
> - tristate "Qcom SCM driver"
> - depends on ARM || ARM64
> - depends on HAVE_ARM_SMCCC
> - select RESET_CONTROLLER
> + tristate
>  
>  config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
>   bool "Qualcomm download mode enabled by default"
> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> index e9c6af78b1d7..3ddf739a6f9b 100644
> --- a/drivers/gpu/drm/msm/Kconfig
> +++ b/drivers/gpu/drm/msm/Kconfig
> @@ -17,7 +17,7 @@ config DRM_MSM
>   select DRM_SCHED
>   select SHMEM
>   select TMPFS
> - select QCOM_SCM if ARCH_QCOM
> + select QCOM_SCM
>   select WANT_DEV_COREDUMP
>   select SND_SOC_HDMI_CODEC if SND_SOC
>   select SYNC_FILE
> @@ -55,7 +55,7 @@ config DRM_MSM_GPU_SUDO
>  
>  config DRM_MSM_HDMI_HDCP
>   bool "Enable HDMI HDCP support in MSM DRM driver"
> - depends on DRM_MSM && QCOM_SCM
> + depends on DRM_MSM
>   default y
>   help
> Choose this option to enable HDCP state machine
> 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

As noted in the RFC, I think you also need to fix QCOM_IOMMU.

In particular (iirc) since all the users of the iommu might be modules,
which would prevent QCOM_IOMMU from being selected.

The rest looks good.

Regards,
Bjorn

> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 157c924686e4..80321e03809a 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -565,7 +565,7 @@ config VIDEO_QCOM_VENUS
>   depends on VIDEO_DEV && VIDEO_V4L2 && QCOM_SMEM
>   depends on (ARCH_QCOM && IOMMU_DMA) || COMPILE_TEST
>   select QCOM_MDT_LOADER if ARCH_QCOM
> - select QCOM_SCM if ARCH_QCOM
> + select QCOM_SCM
>   select VIDEOBUF2_DMA_CONTIG
>   select V4L2_MEM2MEM_DEV
>   help
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 71313961cc54..95b3511b05

Re: [PATCH 2/2] [v2] qcom_scm: hide Kconfig symbol

2021-09-28 Thread Alex Elder via iommu

On 9/28/21 2:50 AM, Arnd Bergmann wrote:

From: Arnd Bergmann 

Now that SCM can be a loadable module, we have to add another
dependency to avoid link failures when ipa or adreno-gpu are
built-in:

aarch64-linux-ld: drivers/net/ipa/ipa_main.o: in function `ipa_probe':
ipa_main.c:(.text+0xfc4): undefined reference to `qcom_scm_is_available'

ld.lld: error: undefined symbol: qcom_scm_is_available

referenced by adreno_gpu.c
   gpu/drm/msm/adreno/adreno_gpu.o:(adreno_zap_shader_load) in 
archive drivers/built-in.a


This can happen when CONFIG_ARCH_QCOM is disabled and we don't select
QCOM_MDT_LOADER, but some other module selects QCOM_SCM. Ideally we'd
use a similar dependency here to what we have for QCOM_RPROC_COMMON,
but that causes dependency loops from other things selecting QCOM_SCM.

This appears to be an endless problem, so try something different this
time:

  - CONFIG_QCOM_SCM becomes a hidden symbol that nothing 'depends on'
but that is simply selected by all of its users

  - All the stubs in include/linux/qcom_scm.h can go away

  - arm-smccc.h needs to provide a stub for __arm_smccc_smc() to
allow compile-testing QCOM_SCM on all architectures.

  - To avoid a circular dependency chain involving RESET_CONTROLLER
and PINCTRL_SUNXI, drop the 'select RESET_CONTROLLER' statement.
According to my testing this still builds fine, and the QCOM
platform selects this symbol already.

Acked-by: Kalle Valo 
Signed-off-by: Arnd Bergmann 
---
Changes in v2:
   - drop the 'select RESET_CONTROLLER' line, rather than adding
 more of the same
---
  drivers/firmware/Kconfig|  5 +-
  drivers/gpu/drm/msm/Kconfig |  4 +-
  drivers/iommu/Kconfig   |  2 +-
  drivers/media/platform/Kconfig  |  2 +-
  drivers/mmc/host/Kconfig|  2 +-
  drivers/net/ipa/Kconfig |  1 +


For drivers/net/ipa/Kconfig, looks good to me.
Nice simplification.

Acked-by: Alex Elder 


  drivers/net/wireless/ath/ath10k/Kconfig |  2 +-
  drivers/pinctrl/qcom/Kconfig|  3 +-
  include/linux/arm-smccc.h   | 10 
  include/linux/qcom_scm.h| 71 -
  10 files changed, 20 insertions(+), 82 deletions(-)



. . .
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 2/2] [v2] qcom_scm: hide Kconfig symbol

2021-09-28 Thread Arnd Bergmann
From: Arnd Bergmann 

Now that SCM can be a loadable module, we have to add another
dependency to avoid link failures when ipa or adreno-gpu are
built-in:

aarch64-linux-ld: drivers/net/ipa/ipa_main.o: in function `ipa_probe':
ipa_main.c:(.text+0xfc4): undefined reference to `qcom_scm_is_available'

ld.lld: error: undefined symbol: qcom_scm_is_available
>>> referenced by adreno_gpu.c
>>>   gpu/drm/msm/adreno/adreno_gpu.o:(adreno_zap_shader_load) in 
>>> archive drivers/built-in.a

This can happen when CONFIG_ARCH_QCOM is disabled and we don't select
QCOM_MDT_LOADER, but some other module selects QCOM_SCM. Ideally we'd
use a similar dependency here to what we have for QCOM_RPROC_COMMON,
but that causes dependency loops from other things selecting QCOM_SCM.

This appears to be an endless problem, so try something different this
time:

 - CONFIG_QCOM_SCM becomes a hidden symbol that nothing 'depends on'
   but that is simply selected by all of its users

 - All the stubs in include/linux/qcom_scm.h can go away

 - arm-smccc.h needs to provide a stub for __arm_smccc_smc() to
   allow compile-testing QCOM_SCM on all architectures.

 - To avoid a circular dependency chain involving RESET_CONTROLLER
   and PINCTRL_SUNXI, drop the 'select RESET_CONTROLLER' statement.
   According to my testing this still builds fine, and the QCOM
   platform selects this symbol already.

Acked-by: Kalle Valo 
Signed-off-by: Arnd Bergmann 
---
Changes in v2:
  - drop the 'select RESET_CONTROLLER' line, rather than adding
more of the same
---
 drivers/firmware/Kconfig|  5 +-
 drivers/gpu/drm/msm/Kconfig |  4 +-
 drivers/iommu/Kconfig   |  2 +-
 drivers/media/platform/Kconfig  |  2 +-
 drivers/mmc/host/Kconfig|  2 +-
 drivers/net/ipa/Kconfig |  1 +
 drivers/net/wireless/ath/ath10k/Kconfig |  2 +-
 drivers/pinctrl/qcom/Kconfig|  3 +-
 include/linux/arm-smccc.h   | 10 
 include/linux/qcom_scm.h| 71 -
 10 files changed, 20 insertions(+), 82 deletions(-)

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 220a58cf0a44..cda7d7162cbb 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -203,10 +203,7 @@ config INTEL_STRATIX10_RSU
  Say Y here if you want Intel RSU support.
 
 config QCOM_SCM
-   tristate "Qcom SCM driver"
-   depends on ARM || ARM64
-   depends on HAVE_ARM_SMCCC
-   select RESET_CONTROLLER
+   tristate
 
 config QCOM_SCM_DOWNLOAD_MODE_DEFAULT
bool "Qualcomm download mode enabled by default"
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index e9c6af78b1d7..3ddf739a6f9b 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -17,7 +17,7 @@ config DRM_MSM
select DRM_SCHED
select SHMEM
select TMPFS
-   select QCOM_SCM if ARCH_QCOM
+   select QCOM_SCM
select WANT_DEV_COREDUMP
select SND_SOC_HDMI_CODEC if SND_SOC
select SYNC_FILE
@@ -55,7 +55,7 @@ config DRM_MSM_GPU_SUDO
 
 config DRM_MSM_HDMI_HDCP
bool "Enable HDMI HDCP support in MSM DRM driver"
-   depends on DRM_MSM && QCOM_SCM
+   depends on DRM_MSM
default y
help
  Choose this option to enable HDCP state machine
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
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 157c924686e4..80321e03809a 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -565,7 +565,7 @@ config VIDEO_QCOM_VENUS
depends on VIDEO_DEV && VIDEO_V4L2 && QCOM_SMEM
depends on (ARCH_QCOM && IOMMU_DMA) || COMPILE_TEST
select QCOM_MDT_LOADER if ARCH_QCOM
-   select QCOM_SCM if ARCH_QCOM
+   select QCOM_SCM
select VIDEOBUF2_DMA_CONTIG
select V4L2_MEM2MEM_DEV
help
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 71313961cc54..95b3511b0560 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -547,7 +547,7 @@ config MMC_SDHCI_MSM
depends on MMC_SDHCI_PLTFM
select MMC_SDHCI_IO_ACCESSORS
select MMC_CQHCI
-   select QCOM_SCM if MMC_CRYPTO && ARCH_QCOM
+   select QCOM_SCM if MMC_CRYPTO
help
  This selects the Secure Digital Host Controller Interface (SDHCI)
  support present in Qualcomm SOCs. The controller