[PATCH] interconnect: Disallow interconnect core to be built as a module

[PATCH] interconnect: Disallow interconnect core to be built as a module

[Viresh Kumar]

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(-)

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

Re: [PATCH] interconnect: Disallow interconnect core to be built as a module

[Bjorn Andersson]

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
>

Re: [PATCH] interconnect: Disallow interconnect core to be built as a module

[Georgi Djakov]

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?

Thanks,
Georgi

[1] https://lore.kernel.org/linux-pm/20191107142111.GB109902@kroah.com/

>>
> 
> 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
>>

Re: [PATCH] interconnect: Disallow interconnect core to be built as a module

[Greg Kroah-Hartman]

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.

thanks,

greg k-h

Re: [PATCH] interconnect: Disallow interconnect core to be built as a module

[Bjorn Andersson]

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

Re: [PATCH] interconnect: Disallow interconnect core to be built as a module

[Viresh Kumar]

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

Re: [PATCH] interconnect: Disallow interconnect core to be built as a module

[Bjorn Andersson]

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.

Regards,
Bjorn

Re: [PATCH] interconnect: Disallow interconnect core to be built as a module

[Viresh Kumar]

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 :)

-- 
viresh