diff mbox series

remoteproc: qcom: fix glink dependencies

Message ID 20210204154010.1585457-1-arnd@kernel.org (mailing list archive)
State Accepted
Commit bfb44502b8fc865c9962ca335ae9877579ff4a9c
Headers show
Series remoteproc: qcom: fix glink dependencies | expand

Commit Message

Arnd Bergmann Feb. 4, 2021, 3:40 p.m. UTC 
From: Arnd Bergmann <arnd@arndb.de>

Building the remoteproc drivers into the kernel while the qcom_glink
code is in a loadable module results in a link error:

ld.lld: error: undefined symbol: qcom_glink_ssr_notify
>>> referenced by vmlinux.o:(glink_subdev_unprepare)

Add a Kconfig dependency to avoid this.

Fixes: 8527efc59d45 ("rpmsg: glink: Guard qcom_glink_ssr_notify() with correct config")
Fixes: 5d1f2e3c8090 ("soc: qcom: glink_ssr: Internalize ssr_notifiers")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/remoteproc/Kconfig | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Alex Elder Feb. 4, 2021, 4:16 p.m. UTC | #1 
On 2/4/21 9:40 AM, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Building the remoteproc drivers into the kernel while the qcom_glink
> code is in a loadable module results in a link error:
> 
> ld.lld: error: undefined symbol: qcom_glink_ssr_notify
>>>> referenced by vmlinux.o:(glink_subdev_unprepare)
> 
> Add a Kconfig dependency to avoid this.

So if they're incompatible in that way it's declared in the
header but not defined in the kernel so the link fails.

And at issue is anything that calls qcom_add_glink_subdev(),
which is qcom_q6v5_{adsp,mss,pas,wcss}.c, and those are
selected by CONFIG_QCOM_Q6V5_{ADSP,MSS,PAS,WCSS}.

But I don't see why QCOM_WCNSS_PIL needs the dependency.

Can you explain that?  Other that that this looks good
to me (and I'll offer reviewed-by with your response).

Thanks.

					-Alex

> Fixes: 8527efc59d45 ("rpmsg: glink: Guard qcom_glink_ssr_notify() with correct config")
> Fixes: 5d1f2e3c8090 ("soc: qcom: glink_ssr: Internalize ssr_notifiers")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   drivers/remoteproc/Kconfig | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 9e7efe542f69..88488af01fd9 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -155,6 +155,7 @@ config QCOM_Q6V5_ADSP
>   	depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
>   	depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
>   	depends on QCOM_SYSMON || QCOM_SYSMON=n
> +	depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
>   	select MFD_SYSCON
>   	select QCOM_PIL_INFO
>   	select QCOM_MDT_LOADER
> @@ -171,6 +172,7 @@ config QCOM_Q6V5_MSS
>   	depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
>   	depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
>   	depends on QCOM_SYSMON || QCOM_SYSMON=n
> +	depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
>   	select MFD_SYSCON
>   	select QCOM_MDT_LOADER
>   	select QCOM_PIL_INFO
> @@ -188,6 +190,7 @@ config QCOM_Q6V5_PAS
>   	depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
>   	depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
>   	depends on QCOM_SYSMON || QCOM_SYSMON=n
> +	depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
>   	select MFD_SYSCON
>   	select QCOM_PIL_INFO
>   	select QCOM_MDT_LOADER
> @@ -206,6 +209,7 @@ config QCOM_Q6V5_WCSS
>   	depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
>   	depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
>   	depends on QCOM_SYSMON || QCOM_SYSMON=n
> +	depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
>   	select MFD_SYSCON
>   	select QCOM_MDT_LOADER
>   	select QCOM_PIL_INFO
> @@ -238,6 +242,7 @@ config QCOM_WCNSS_PIL
>   	depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
>   	depends on QCOM_SMEM
>   	depends on QCOM_SYSMON || QCOM_SYSMON=n
> +	depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
>   	select QCOM_MDT_LOADER
>   	select QCOM_PIL_INFO
>   	select QCOM_RPROC_COMMON
>
Arnd Bergmann Feb. 4, 2021, 5:16 p.m. UTC | #2 
On Thu, Feb 4, 2021 at 5:16 PM Alex Elder <elder@linaro.org> wrote:
>
> On 2/4/21 9:40 AM, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > Building the remoteproc drivers into the kernel while the qcom_glink
> > code is in a loadable module results in a link error:
> >
> > ld.lld: error: undefined symbol: qcom_glink_ssr_notify
> >>>> referenced by vmlinux.o:(glink_subdev_unprepare)
> >
> > Add a Kconfig dependency to avoid this.
>
> So if they're incompatible in that way it's declared in the
> header but not defined in the kernel so the link fails.
>
> And at issue is anything that calls qcom_add_glink_subdev(),
> which is qcom_q6v5_{adsp,mss,pas,wcss}.c, and those are
> selected by CONFIG_QCOM_Q6V5_{ADSP,MSS,PAS,WCSS}.
>
> But I don't see why QCOM_WCNSS_PIL needs the dependency.
>
> Can you explain that?  Other that that this looks good
> to me (and I'll offer reviewed-by with your response).

The problem is that QCOM_WCNSS_PIL selects
QCOM_RPROC_COMMON, so qcom_common.o is built-in
whenever QCOM_WCNSS_PIL=y, and this will lead to the
same link failure, regardless of whether the function is needed
or not.

       Arnd
Alex Elder Feb. 4, 2021, 5:32 p.m. UTC | #3 
On 2/4/21 11:16 AM, Arnd Bergmann wrote:
> On Thu, Feb 4, 2021 at 5:16 PM Alex Elder <elder@linaro.org> wrote:
>>
>> On 2/4/21 9:40 AM, Arnd Bergmann wrote:
>>> From: Arnd Bergmann <arnd@arndb.de>
>>>
>>> Building the remoteproc drivers into the kernel while the qcom_glink
>>> code is in a loadable module results in a link error:
>>>
>>> ld.lld: error: undefined symbol: qcom_glink_ssr_notify
>>>>>> referenced by vmlinux.o:(glink_subdev_unprepare)
>>>
>>> Add a Kconfig dependency to avoid this.
>>
>> So if they're incompatible in that way it's declared in the
>> header but not defined in the kernel so the link fails.
>>
>> And at issue is anything that calls qcom_add_glink_subdev(),
>> which is qcom_q6v5_{adsp,mss,pas,wcss}.c, and those are
>> selected by CONFIG_QCOM_Q6V5_{ADSP,MSS,PAS,WCSS}.
>>
>> But I don't see why QCOM_WCNSS_PIL needs the dependency.
>>
>> Can you explain that?  Other that that this looks good
>> to me (and I'll offer reviewed-by with your response).
> 
> The problem is that QCOM_WCNSS_PIL selects
> QCOM_RPROC_COMMON, so qcom_common.o is built-in
> whenever QCOM_WCNSS_PIL=y, and this will lead to the
> same link failure, regardless of whether the function is needed
> or not.

Thanks for the explanation.  Maybe that is something
than get fixed, maybe not.

Reviewed-by: Alex Elder <elder@linaro.org>

>        Arnd
>
patchwork-bot+linux-remoteproc@kernel.org Feb. 9, 2021, 5:40 p.m. UTC | #4 
Hello:

This patch was applied to andersson/remoteproc.git (refs/heads/for-next):

On Thu,  4 Feb 2021 16:40:04 +0100 you wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Building the remoteproc drivers into the kernel while the qcom_glink
> code is in a loadable module results in a link error:
> 
> ld.lld: error: undefined symbol: qcom_glink_ssr_notify
> >>> referenced by vmlinux.o:(glink_subdev_unprepare)
> 
> [...]

Here is the summary with links:
  - remoteproc: qcom: fix glink dependencies
    https://git.kernel.org/andersson/remoteproc/c/bfb44502b8fc

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
diff mbox series

Patch

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 9e7efe542f69..88488af01fd9 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -155,6 +155,7 @@  config QCOM_Q6V5_ADSP
 	depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
 	depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
 	depends on QCOM_SYSMON || QCOM_SYSMON=n
+	depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
 	select MFD_SYSCON
 	select QCOM_PIL_INFO
 	select QCOM_MDT_LOADER
@@ -171,6 +172,7 @@  config QCOM_Q6V5_MSS
 	depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
 	depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
 	depends on QCOM_SYSMON || QCOM_SYSMON=n
+	depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
 	select MFD_SYSCON
 	select QCOM_MDT_LOADER
 	select QCOM_PIL_INFO
@@ -188,6 +190,7 @@  config QCOM_Q6V5_PAS
 	depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
 	depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
 	depends on QCOM_SYSMON || QCOM_SYSMON=n
+	depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
 	select MFD_SYSCON
 	select QCOM_PIL_INFO
 	select QCOM_MDT_LOADER
@@ -206,6 +209,7 @@  config QCOM_Q6V5_WCSS
 	depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
 	depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
 	depends on QCOM_SYSMON || QCOM_SYSMON=n
+	depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
 	select MFD_SYSCON
 	select QCOM_MDT_LOADER
 	select QCOM_PIL_INFO
@@ -238,6 +242,7 @@  config QCOM_WCNSS_PIL
 	depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
 	depends on QCOM_SMEM
 	depends on QCOM_SYSMON || QCOM_SYSMON=n
+	depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
 	select QCOM_MDT_LOADER
 	select QCOM_PIL_INFO
 	select QCOM_RPROC_COMMON