cpufreq: kryo: allow building as a loadable module
diff mbox

Message ID 20180605114453.1381941-1-arnd@arndb.de
State Mainlined
Delegated to: Rafael Wysocki
Headers show

Commit Message

Arnd Bergmann June 5, 2018, 11:44 a.m. UTC
Building the kryo cpufreq driver while QCOM_SMEM is a loadable module
results in a link error:

drivers/cpufreq/qcom-cpufreq-kryo.o: In function `qcom_cpufreq_kryo_probe':
qcom-cpufreq-kryo.c:(.text+0xbc): undefined reference to `qcom_smem_get'

The problem is that Kconfig ignores interprets the dependency as met
when the dependent symbol is a 'bool' one. By making it 'tristate',
it will be forced to be a module here, which builds successfully.

Fixes: 46e2856b8e18 ("cpufreq: Add Kryo CPU scaling driver")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/cpufreq/Kconfig.arm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Viresh Kumar June 6, 2018, 4:09 a.m. UTC | #1
On 05-06-18, 13:44, Arnd Bergmann wrote:
> Building the kryo cpufreq driver while QCOM_SMEM is a loadable module
> results in a link error:
> 
> drivers/cpufreq/qcom-cpufreq-kryo.o: In function `qcom_cpufreq_kryo_probe':
> qcom-cpufreq-kryo.c:(.text+0xbc): undefined reference to `qcom_smem_get'
> 
> The problem is that Kconfig ignores interprets the dependency as met
> when the dependent symbol is a 'bool' one. By making it 'tristate',
> it will be forced to be a module here, which builds successfully.
> 
> Fixes: 46e2856b8e18 ("cpufreq: Add Kryo CPU scaling driver")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/cpufreq/Kconfig.arm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index c7ce928fbf1f..52f5f1a2040c 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -125,7 +125,7 @@ config ARM_OMAP2PLUS_CPUFREQ
>  	default ARCH_OMAP2PLUS
>  
>  config ARM_QCOM_CPUFREQ_KRYO
> -	bool "Qualcomm Kryo based CPUFreq"
> +	tristate "Qualcomm Kryo based CPUFreq"
>  	depends on ARM64
>  	depends on QCOM_QFPROM
>  	depends on QCOM_SMEM

Okay, so we really need this to be a module. But the driver can't really work as
a module right now if we do this: insmod, rmmod, insmod. Because it doesn't free
resources at rmmmod and will fail on second insmod.

Because what you are fixing is a critical build error, we better get it merged
right now.

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

But Ilia needs to cook another patch to add the module removal code for the
driver and mark your patch's commit id in the fixes tag.
Ilia Lin June 6, 2018, 4:44 a.m. UTC | #2
Viresh got ahead of my answer a bit. :)
Sure I'll post the module exit later.

On June 6, 2018 7:09:29 AM GMT+03:00, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>On 05-06-18, 13:44, Arnd Bergmann wrote:
>> Building the kryo cpufreq driver while QCOM_SMEM is a loadable module
>> results in a link error:
>> 
>> drivers/cpufreq/qcom-cpufreq-kryo.o: In function
>`qcom_cpufreq_kryo_probe':
>> qcom-cpufreq-kryo.c:(.text+0xbc): undefined reference to
>`qcom_smem_get'
>> 
>> The problem is that Kconfig ignores interprets the dependency as met
>> when the dependent symbol is a 'bool' one. By making it 'tristate',
>> it will be forced to be a module here, which builds successfully.
>> 
>> Fixes: 46e2856b8e18 ("cpufreq: Add Kryo CPU scaling driver")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>>  drivers/cpufreq/Kconfig.arm | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/cpufreq/Kconfig.arm
>b/drivers/cpufreq/Kconfig.arm
>> index c7ce928fbf1f..52f5f1a2040c 100644
>> --- a/drivers/cpufreq/Kconfig.arm
>> +++ b/drivers/cpufreq/Kconfig.arm
>> @@ -125,7 +125,7 @@ config ARM_OMAP2PLUS_CPUFREQ
>>  	default ARCH_OMAP2PLUS
>>  
>>  config ARM_QCOM_CPUFREQ_KRYO
>> -	bool "Qualcomm Kryo based CPUFreq"
>> +	tristate "Qualcomm Kryo based CPUFreq"
>>  	depends on ARM64
>>  	depends on QCOM_QFPROM
>>  	depends on QCOM_SMEM
>
>Okay, so we really need this to be a module. But the driver can't
>really work as
>a module right now if we do this: insmod, rmmod, insmod. Because it
>doesn't free
>resources at rmmmod and will fail on second insmod.
>
>Because what you are fixing is a critical build error, we better get it
>merged
>right now.
>
>Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>
>But Ilia needs to cook another patch to add the module removal code for
>the
>driver and mark your patch's commit id in the fixes tag.
Rafael J. Wysocki June 12, 2018, 3:02 p.m. UTC | #3
On Wednesday, June 6, 2018 6:09:29 AM CEST Viresh Kumar wrote:
> On 05-06-18, 13:44, Arnd Bergmann wrote:
> > Building the kryo cpufreq driver while QCOM_SMEM is a loadable module
> > results in a link error:
> > 
> > drivers/cpufreq/qcom-cpufreq-kryo.o: In function `qcom_cpufreq_kryo_probe':
> > qcom-cpufreq-kryo.c:(.text+0xbc): undefined reference to `qcom_smem_get'
> > 
> > The problem is that Kconfig ignores interprets the dependency as met
> > when the dependent symbol is a 'bool' one. By making it 'tristate',
> > it will be forced to be a module here, which builds successfully.
> > 
> > Fixes: 46e2856b8e18 ("cpufreq: Add Kryo CPU scaling driver")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  drivers/cpufreq/Kconfig.arm | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> > index c7ce928fbf1f..52f5f1a2040c 100644
> > --- a/drivers/cpufreq/Kconfig.arm
> > +++ b/drivers/cpufreq/Kconfig.arm
> > @@ -125,7 +125,7 @@ config ARM_OMAP2PLUS_CPUFREQ
> >  	default ARCH_OMAP2PLUS
> >  
> >  config ARM_QCOM_CPUFREQ_KRYO
> > -	bool "Qualcomm Kryo based CPUFreq"
> > +	tristate "Qualcomm Kryo based CPUFreq"
> >  	depends on ARM64
> >  	depends on QCOM_QFPROM
> >  	depends on QCOM_SMEM
> 
> Okay, so we really need this to be a module. But the driver can't really work as
> a module right now if we do this: insmod, rmmod, insmod. Because it doesn't free
> resources at rmmmod and will fail on second insmod.
> 
> Because what you are fixing is a critical build error, we better get it merged
> right now.
> 
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Patch applied, thanks!

Patch
diff mbox

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index c7ce928fbf1f..52f5f1a2040c 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -125,7 +125,7 @@  config ARM_OMAP2PLUS_CPUFREQ
 	default ARCH_OMAP2PLUS
 
 config ARM_QCOM_CPUFREQ_KRYO
-	bool "Qualcomm Kryo based CPUFreq"
+	tristate "Qualcomm Kryo based CPUFreq"
 	depends on ARM64
 	depends on QCOM_QFPROM
 	depends on QCOM_SMEM