diff mbox series

interconnect: Disallow interconnect core to be built as a module

Message ID b789cce388dd1f2906492f307dea6780c398bc6a.1567065991.git.viresh.kumar@linaro.org
State Accepted
Commit fcb57bfcb87f3bdb1b29fea1a1cd72940fa559fd
Headers show
Series interconnect: Disallow interconnect core to be built as a module | expand

Commit Message

Viresh Kumar Aug. 29, 2019, 8:07 a.m. UTC
Building individual drivers as modules is fine but allowing a core
framework to be built as a module makes it really complex and should be
avoided.

Whatever uses the interconnect core APIs must also be built as a module
if interconnect core is built as module, else we will see compilation
failures.

If another core framework (like cpufreq, clk, etc), that can't be built
as module, needs to use interconnect APIs then we will start seeing
compilation failures with allmodconfig configurations as the symbols
(like of_icc_get()) used in other frameworks will not be available in
the built-in image.

Disallow the interconnect core to be built as a module to avoid all
these issues.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

---
 drivers/interconnect/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.21.0.rc0.269.g1a574e7a288b

Comments

Bjorn Andersson Sept. 12, 2019, 4:33 p.m. UTC | #1
On Thu, Aug 29, 2019 at 1:07 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>

> Building individual drivers as modules is fine but allowing a core

> framework to be built as a module makes it really complex and should be

> avoided.

>

> Whatever uses the interconnect core APIs must also be built as a module

> if interconnect core is built as module, else we will see compilation

> failures.

>

> If another core framework (like cpufreq, clk, etc), that can't be built

> as module, needs to use interconnect APIs then we will start seeing

> compilation failures with allmodconfig configurations as the symbols

> (like of_icc_get()) used in other frameworks will not be available in

> the built-in image.

>

> Disallow the interconnect core to be built as a module to avoid all

> these issues.

>


Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>


> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

> ---

>  drivers/interconnect/Kconfig | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig

> index bfa4ca3ab7a9..b6ea8f0a6122 100644

> --- a/drivers/interconnect/Kconfig

> +++ b/drivers/interconnect/Kconfig

> @@ -1,6 +1,6 @@

>  # SPDX-License-Identifier: GPL-2.0-only

>  menuconfig INTERCONNECT

> -       tristate "On-Chip Interconnect management support"

> +       bool "On-Chip Interconnect management support"

>         help

>           Support for management of the on-chip interconnects.

>

> --

> 2.21.0.rc0.269.g1a574e7a288b

>
Bjorn Andersson May 18, 2020, 6:40 p.m. UTC | #2
On Fri 15 May 00:11 PDT 2020, Greg Kroah-Hartman wrote:

> On Fri, May 15, 2020 at 07:48:47AM +0300, Georgi Djakov wrote:
> > On 9/12/19 19:33, Bjorn Andersson wrote:
> > > On Thu, Aug 29, 2019 at 1:07 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > >>
> > >> Building individual drivers as modules is fine but allowing a core
> > >> framework to be built as a module makes it really complex and should be
> > >> avoided.
> > >>
> > >> Whatever uses the interconnect core APIs must also be built as a module
> > >> if interconnect core is built as module, else we will see compilation
> > >> failures.
> > >>
> > >> If another core framework (like cpufreq, clk, etc), that can't be built
> > >> as module, needs to use interconnect APIs then we will start seeing
> > >> compilation failures with allmodconfig configurations as the symbols
> > >> (like of_icc_get()) used in other frameworks will not be available in
> > >> the built-in image.
> > >>
> > >> Disallow the interconnect core to be built as a module to avoid all
> > >> these issues.
> > 
> > Hi Greg,
> > 
> > We had a discussion [1] a few months back about frameworks being built as
> > modules. IIUC, you initially expressed some doubts about this patch, so i
> > wanted to check with you again on this.
> > 
> > While i think that the possibility for a framework core to be a module is a
> > nice feature, and we should try to be as modular as possible, it seems that
> > handling dependencies between the different core frameworks becomes difficult
> > when one of them is tristate.
> > 
> > This of course affects the drivers which use it (every client should express
> > the dependency in Kconfig as a "depends on framework || !framework"), in order
> > to avoid build failures in the case when framework=m and client=y. However, this
> > is not a big issue.
> > 
> > But it gets more complex when another framework2 becomes a client of the modular
> > framework and especially when framework2 is "select"-ed in Kconfig by it's
> > users. When selects are used in Kconfig, it forces the option, without ever
> > visiting the dependencies. I am not sure what we should do in this case, maybe
> > we can continue and sprinkle more "depends on framework || !framework" also for
> > every single user which selects framework2.. But i believe that this is very
> > inconvenient.
> > 
> > Well, the above is not impossible, but other frameworks (regulator, clk, reset,
> > pinctrl, etc.) are solving this problem by just being bool, instead of tristate.
> > This makes life much easier for everyone. So i am wondering if it wouldn't be
> > more appropriate to use the same approach here too?
> 
> Ok, if it makes things easier, perhaps this is the best way to handle
> it.
> 

It most certainly does.

With INTERCONNECT as a bool we can handle its absence with stub
functions - like every other framework does. But as a tristate then
every driver with a call to the interconnect api needs an entry in
Kconfig to ensure the client driver must be a module if the interconnect
framework is.

Regards,
Bjorn
Viresh Kumar May 19, 2020, 3:31 a.m. UTC | #3
On 18-05-20, 11:40, Bjorn Andersson wrote:
> It most certainly does.
> 
> With INTERCONNECT as a bool we can handle its absence with stub
> functions - like every other framework does. But as a tristate then
> every driver with a call to the interconnect api needs an entry in
> Kconfig to ensure the client driver must be a module if the interconnect
> framework is.

This patch has been pushed to linux-next a few days back.
Viresh Kumar May 19, 2020, 3:42 a.m. UTC | #4
On 18-05-20, 20:37, Bjorn Andersson wrote:
> On Mon 18 May 20:31 PDT 2020, Viresh Kumar wrote:
> 
> > On 18-05-20, 11:40, Bjorn Andersson wrote:
> > > It most certainly does.
> > > 
> > > With INTERCONNECT as a bool we can handle its absence with stub
> > > functions - like every other framework does. But as a tristate then
> > > every driver with a call to the interconnect api needs an entry in
> > > Kconfig to ensure the client driver must be a module if the interconnect
> > > framework is.
> > 
> > This patch has been pushed to linux-next a few days back.
> > 
> 
> Thanks Viresh, I had missed that.

Not your fault, we didn't resend it but simply applied the old version
itself :)
diff mbox series

Patch

diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
index bfa4ca3ab7a9..b6ea8f0a6122 100644
--- a/drivers/interconnect/Kconfig
+++ b/drivers/interconnect/Kconfig
@@ -1,6 +1,6 @@ 
 # SPDX-License-Identifier: GPL-2.0-only
 menuconfig INTERCONNECT
-	tristate "On-Chip Interconnect management support"
+	bool "On-Chip Interconnect management support"
 	help
 	  Support for management of the on-chip interconnects.