* [PATCH] interconnect: qcom: fix rpmh link failures
@ 2020-12-04 16:50 Arnd Bergmann
2020-12-04 23:27 ` Bjorn Andersson
0 siblings, 1 reply; 3+ messages in thread
From: Arnd Bergmann @ 2020-12-04 16:50 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Georgi Djakov
Cc: Arnd Bergmann, Sibi Sankar, Evan Green, Jonathan Marek,
Odelu Kukatla, Krzysztof Kozlowski, David Dai, linux-arm-msm,
linux-pm, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
When CONFIG_COMPILE_TEST is set, it is possible to build some
of the interconnect drivers into the kernel while their dependencies
are loadable modules, which is bad:
arm-linux-gnueabi-ld: drivers/interconnect/qcom/bcm-voter.o: in function `qcom_icc_bcm_voter_commit':
(.text+0x1f8): undefined reference to `rpmh_invalidate'
arm-linux-gnueabi-ld: (.text+0x20c): undefined reference to `rpmh_write_batch'
arm-linux-gnueabi-ld: (.text+0x2b0): undefined reference to `rpmh_write_batch'
arm-linux-gnueabi-ld: (.text+0x2e8): undefined reference to `rpmh_write_batch'
arm-linux-gnueabi-ld: drivers/interconnect/qcom/icc-rpmh.o: in function `qcom_icc_bcm_init':
(.text+0x2ac): undefined reference to `cmd_db_read_addr'
arm-linux-gnueabi-ld: (.text+0x2c8): undefined reference to `cmd_db_read_aux_data'
The exact dependencies are a bit complicated, so split them out into a
hidden Kconfig symbol that all drivers can in turn depend on to get it
right.
Fixes: 976daac4a1c5 ("interconnect: qcom: Consolidate interconnect RPMh support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/interconnect/qcom/Kconfig | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
index a8f93ba265f8..b3fb5b02bcf1 100644
--- a/drivers/interconnect/qcom/Kconfig
+++ b/drivers/interconnect/qcom/Kconfig
@@ -42,13 +42,23 @@ config INTERCONNECT_QCOM_QCS404
This is a driver for the Qualcomm Network-on-Chip on qcs404-based
platforms.
+config INTERCONNECT_QCOM_RPMH_POSSIBLE
+ tristate
+ default INTERCONNECT_QCOM
+ depends on QCOM_RPMH || (COMPILE_TEST && !QCOM_RPMH)
+ depends on QCOM_COMMAND_DB || (COMPILE_TEST && !QCOM_COMMAND_DB)
+ depends on OF || COMPILE_TEST
+ help
+ Compile-testing RPMH drivers is possible on other platforms,
+ but in order to avoid link failures, drivers must not be built-in
+ when QCOM_RPMH or QCOM_COMMAND_DB are loadable modules
+
config INTERCONNECT_QCOM_RPMH
tristate
config INTERCONNECT_QCOM_SC7180
tristate "Qualcomm SC7180 interconnect driver"
- depends on INTERCONNECT_QCOM
- depends on (QCOM_RPMH && QCOM_COMMAND_DB && OF) || COMPILE_TEST
+ depends on INTERCONNECT_QCOM_RPMH_POSSIBLE
select INTERCONNECT_QCOM_RPMH
select INTERCONNECT_QCOM_BCM_VOTER
help
@@ -57,8 +67,7 @@ config INTERCONNECT_QCOM_SC7180
config INTERCONNECT_QCOM_SDM845
tristate "Qualcomm SDM845 interconnect driver"
- depends on INTERCONNECT_QCOM
- depends on (QCOM_RPMH && QCOM_COMMAND_DB && OF) || COMPILE_TEST
+ depends on INTERCONNECT_QCOM_RPMH_POSSIBLE
select INTERCONNECT_QCOM_RPMH
select INTERCONNECT_QCOM_BCM_VOTER
help
@@ -67,8 +76,7 @@ config INTERCONNECT_QCOM_SDM845
config INTERCONNECT_QCOM_SM8150
tristate "Qualcomm SM8150 interconnect driver"
- depends on INTERCONNECT_QCOM
- depends on (QCOM_RPMH && QCOM_COMMAND_DB && OF) || COMPILE_TEST
+ depends on INTERCONNECT_QCOM_RPMH_POSSIBLE
select INTERCONNECT_QCOM_RPMH
select INTERCONNECT_QCOM_BCM_VOTER
help
@@ -77,8 +85,7 @@ config INTERCONNECT_QCOM_SM8150
config INTERCONNECT_QCOM_SM8250
tristate "Qualcomm SM8250 interconnect driver"
- depends on INTERCONNECT_QCOM
- depends on (QCOM_RPMH && QCOM_COMMAND_DB && OF) || COMPILE_TEST
+ depends on INTERCONNECT_QCOM_RPMH_POSSIBLE
select INTERCONNECT_QCOM_RPMH
select INTERCONNECT_QCOM_BCM_VOTER
help
--
2.27.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] interconnect: qcom: fix rpmh link failures
2020-12-04 16:50 [PATCH] interconnect: qcom: fix rpmh link failures Arnd Bergmann
@ 2020-12-04 23:27 ` Bjorn Andersson
2020-12-05 12:03 ` Arnd Bergmann
0 siblings, 1 reply; 3+ messages in thread
From: Bjorn Andersson @ 2020-12-04 23:27 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andy Gross, Georgi Djakov, Arnd Bergmann, Sibi Sankar,
Evan Green, Jonathan Marek, Odelu Kukatla, Krzysztof Kozlowski,
David Dai, linux-arm-msm, linux-pm, linux-kernel
On Fri 04 Dec 10:50 CST 2020, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> When CONFIG_COMPILE_TEST is set, it is possible to build some
> of the interconnect drivers into the kernel while their dependencies
> are loadable modules, which is bad:
>
> arm-linux-gnueabi-ld: drivers/interconnect/qcom/bcm-voter.o: in function `qcom_icc_bcm_voter_commit':
> (.text+0x1f8): undefined reference to `rpmh_invalidate'
> arm-linux-gnueabi-ld: (.text+0x20c): undefined reference to `rpmh_write_batch'
> arm-linux-gnueabi-ld: (.text+0x2b0): undefined reference to `rpmh_write_batch'
> arm-linux-gnueabi-ld: (.text+0x2e8): undefined reference to `rpmh_write_batch'
> arm-linux-gnueabi-ld: drivers/interconnect/qcom/icc-rpmh.o: in function `qcom_icc_bcm_init':
> (.text+0x2ac): undefined reference to `cmd_db_read_addr'
> arm-linux-gnueabi-ld: (.text+0x2c8): undefined reference to `cmd_db_read_aux_data'
>
> The exact dependencies are a bit complicated, so split them out into a
> hidden Kconfig symbol that all drivers can in turn depend on to get it
> right.
>
> Fixes: 976daac4a1c5 ("interconnect: qcom: Consolidate interconnect RPMh support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Your patch looks correct to me, so:
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
But we're going to have to sprinkle a handful of these throughout the
tree and we're not a lot of people who "understand" what it does (and at
least I keep getting them wrong...)
Perhaps it would be more reasonable to maintain this long term if we
drop the possibility of compile testing these drivers independently of
rpmh and command db? (I.e. drop the function stubs and rely on
RPMH/COMMAND_DB to enable building under COMPILE_TEST)?
And just to make it clear, I think we should merge your patch to fix
v5.11; then consider to simplify this past that.
Regards,
Bjorn
> ---
> drivers/interconnect/qcom/Kconfig | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
> index a8f93ba265f8..b3fb5b02bcf1 100644
> --- a/drivers/interconnect/qcom/Kconfig
> +++ b/drivers/interconnect/qcom/Kconfig
> @@ -42,13 +42,23 @@ config INTERCONNECT_QCOM_QCS404
> This is a driver for the Qualcomm Network-on-Chip on qcs404-based
> platforms.
>
> +config INTERCONNECT_QCOM_RPMH_POSSIBLE
> + tristate
> + default INTERCONNECT_QCOM
> + depends on QCOM_RPMH || (COMPILE_TEST && !QCOM_RPMH)
> + depends on QCOM_COMMAND_DB || (COMPILE_TEST && !QCOM_COMMAND_DB)
> + depends on OF || COMPILE_TEST
> + help
> + Compile-testing RPMH drivers is possible on other platforms,
> + but in order to avoid link failures, drivers must not be built-in
> + when QCOM_RPMH or QCOM_COMMAND_DB are loadable modules
> +
> config INTERCONNECT_QCOM_RPMH
> tristate
>
> config INTERCONNECT_QCOM_SC7180
> tristate "Qualcomm SC7180 interconnect driver"
> - depends on INTERCONNECT_QCOM
> - depends on (QCOM_RPMH && QCOM_COMMAND_DB && OF) || COMPILE_TEST
> + depends on INTERCONNECT_QCOM_RPMH_POSSIBLE
> select INTERCONNECT_QCOM_RPMH
> select INTERCONNECT_QCOM_BCM_VOTER
> help
> @@ -57,8 +67,7 @@ config INTERCONNECT_QCOM_SC7180
>
> config INTERCONNECT_QCOM_SDM845
> tristate "Qualcomm SDM845 interconnect driver"
> - depends on INTERCONNECT_QCOM
> - depends on (QCOM_RPMH && QCOM_COMMAND_DB && OF) || COMPILE_TEST
> + depends on INTERCONNECT_QCOM_RPMH_POSSIBLE
> select INTERCONNECT_QCOM_RPMH
> select INTERCONNECT_QCOM_BCM_VOTER
> help
> @@ -67,8 +76,7 @@ config INTERCONNECT_QCOM_SDM845
>
> config INTERCONNECT_QCOM_SM8150
> tristate "Qualcomm SM8150 interconnect driver"
> - depends on INTERCONNECT_QCOM
> - depends on (QCOM_RPMH && QCOM_COMMAND_DB && OF) || COMPILE_TEST
> + depends on INTERCONNECT_QCOM_RPMH_POSSIBLE
> select INTERCONNECT_QCOM_RPMH
> select INTERCONNECT_QCOM_BCM_VOTER
> help
> @@ -77,8 +85,7 @@ config INTERCONNECT_QCOM_SM8150
>
> config INTERCONNECT_QCOM_SM8250
> tristate "Qualcomm SM8250 interconnect driver"
> - depends on INTERCONNECT_QCOM
> - depends on (QCOM_RPMH && QCOM_COMMAND_DB && OF) || COMPILE_TEST
> + depends on INTERCONNECT_QCOM_RPMH_POSSIBLE
> select INTERCONNECT_QCOM_RPMH
> select INTERCONNECT_QCOM_BCM_VOTER
> help
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] interconnect: qcom: fix rpmh link failures
2020-12-04 23:27 ` Bjorn Andersson
@ 2020-12-05 12:03 ` Arnd Bergmann
0 siblings, 0 replies; 3+ messages in thread
From: Arnd Bergmann @ 2020-12-05 12:03 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Andy Gross, Georgi Djakov, Arnd Bergmann, Sibi Sankar,
Evan Green, Jonathan Marek, Odelu Kukatla, Krzysztof Kozlowski,
David Dai, linux-arm-msm, Linux PM list, linux-kernel
On Sat, Dec 5, 2020 at 12:27 AM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Fri 04 Dec 10:50 CST 2020, Arnd Bergmann wrote:
>
> Your patch looks correct to me, so:
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>
>
> But we're going to have to sprinkle a handful of these throughout the
> tree and we're not a lot of people who "understand" what it does (and at
> least I keep getting them wrong...)
Right, I already ran into another one.
> Perhaps it would be more reasonable to maintain this long term if we
> drop the possibility of compile testing these drivers independently of
> rpmh and command db? (I.e. drop the function stubs and rely on
> RPMH/COMMAND_DB to enable building under COMPILE_TEST)?
Agreed, I think that would be best. As long as RPMH and COMMAND_DB
can individually be enabled for CONFIG_COMPILE_TEST, all the
drivers will get enabled in an allmodconfig build, and we can just
list the dependencies.
I don't really like the headers too much that have an
#ifdef CONFIG_SUBSYSTEM with an alternative set of
inline functions, unless there is a reasonable expectations that
drivers work fine if that subsystem is disabled.
E.g. having the option to disable IPv6 makes sense despite
the complexity that adds, but compile-testing a driver without
a required subsystem that can also be enabled for compile-testing
hurts more than it helps.
> And just to make it clear, I think we should merge your patch to fix
> v5.11; then consider to simplify this past that.
Yes, sounds good.
Arnd
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-12-05 18:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-04 16:50 [PATCH] interconnect: qcom: fix rpmh link failures Arnd Bergmann
2020-12-04 23:27 ` Bjorn Andersson
2020-12-05 12:03 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).