[PATCH v2] cpufreq: qcom-kryo: Fix section mismatch warning

[PATCH v2] cpufreq: qcom-kryo: Fix section mismatch warning

[Nathan Chancellor]

WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from
the function qcom_cpufreq_kryo_probe() to the function
.init.text:qcom_cpufreq_kryo_get_msm_id()
The function qcom_cpufreq_kryo_probe() references
the function __init qcom_cpufreq_kryo_get_msm_id().
This is often because qcom_cpufreq_kryo_probe lacks a __init
annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong.

Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is
no more mismatch warning.

Fixes: 46e2856b8e18 ("cpufreq: Add Kryo CPU scaling driver")
Reported-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---

v1 -> v2:

Add '__init' to qcom_cpufreq_kryo_probe instead of removing it from
qcom_cpufreq_kryo_get_msm_id.

 drivers/cpufreq/qcom-cpufreq-kryo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c b/drivers/cpufreq/qcom-cpufreq-kryo.c
index a1830fa25fc5..e001ef8ea7de 100644
--- a/drivers/cpufreq/qcom-cpufreq-kryo.c
+++ b/drivers/cpufreq/qcom-cpufreq-kryo.c
@@ -73,7 +73,7 @@ static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void)
 	return version;
 }
 
-static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
+static int __init qcom_cpufreq_kryo_probe(struct platform_device *pdev)
 {
 	struct opp_table *opp_tables[NR_CPUS] = {0};
 	enum _msm8996_version msm8996_version;
-- 
2.19.0

Re: [PATCH v2] cpufreq: qcom-kryo: Fix section mismatch warning

[Viresh Kumar]

On 19-09-18, 11:22, Nathan Chancellor wrote:
> WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from
> the function qcom_cpufreq_kryo_probe() to the function
> .init.text:qcom_cpufreq_kryo_get_msm_id()
> The function qcom_cpufreq_kryo_probe() references
> the function __init qcom_cpufreq_kryo_get_msm_id().
> This is often because qcom_cpufreq_kryo_probe lacks a __init
> annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong.
> 
> Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is
> no more mismatch warning.
> 
> Fixes: 46e2856b8e18 ("cpufreq: Add Kryo CPU scaling driver")
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> 
> v1 -> v2:
> 
> Add '__init' to qcom_cpufreq_kryo_probe instead of removing it from
> qcom_cpufreq_kryo_get_msm_id.
> 
>  drivers/cpufreq/qcom-cpufreq-kryo.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c b/drivers/cpufreq/qcom-cpufreq-kryo.c
> index a1830fa25fc5..e001ef8ea7de 100644
> --- a/drivers/cpufreq/qcom-cpufreq-kryo.c
> +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c
> @@ -73,7 +73,7 @@ static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void)
>  	return version;
>  }
>  
> -static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
> +static int __init qcom_cpufreq_kryo_probe(struct platform_device *pdev)
>  {
>  	struct opp_table *opp_tables[NR_CPUS] = {0};
>  	enum _msm8996_version msm8996_version;

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

-- 
viresh

Re: [PATCH v2] cpufreq: qcom-kryo: Fix section mismatch warning

[Nick Desaulniers]

On Wed, Sep 19, 2018 at 11:22 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from
> the function qcom_cpufreq_kryo_probe() to the function
> .init.text:qcom_cpufreq_kryo_get_msm_id()
> The function qcom_cpufreq_kryo_probe() references
> the function __init qcom_cpufreq_kryo_get_msm_id().
> This is often because qcom_cpufreq_kryo_probe lacks a __init
> annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong.
>
> Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is
> no more mismatch warning.

I wonder why this driver has an exit function marked __init rather
than __exit? Does that mean it gets cleaned up after kernel init, and
so on unloading of the driver, the kernel jumps to unmapped memory?

Does this patch now produce a warning for `qcom_cpufreq_kryo_driver`
referencing `qcom_cpufreq_kryo_probe`?

>
> Fixes: 46e2856b8e18 ("cpufreq: Add Kryo CPU scaling driver")
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> v1 -> v2:
>
> Add '__init' to qcom_cpufreq_kryo_probe instead of removing it from
> qcom_cpufreq_kryo_get_msm_id.
>
>  drivers/cpufreq/qcom-cpufreq-kryo.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c b/drivers/cpufreq/qcom-cpufreq-kryo.c
> index a1830fa25fc5..e001ef8ea7de 100644
> --- a/drivers/cpufreq/qcom-cpufreq-kryo.c
> +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c
> @@ -73,7 +73,7 @@ static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void)
>         return version;
>  }
>
> -static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
> +static int __init qcom_cpufreq_kryo_probe(struct platform_device *pdev)
>  {
>         struct opp_table *opp_tables[NR_CPUS] = {0};
>         enum _msm8996_version msm8996_version;
> --
> 2.19.0
>


--
Thanks,
~Nick Desaulniers

Re: [PATCH v2] cpufreq: qcom-kryo: Fix section mismatch warning

[Viresh Kumar]

On 19-09-18, 14:45, Nick Desaulniers wrote:
> On Wed, Sep 19, 2018 at 11:22 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from
> > the function qcom_cpufreq_kryo_probe() to the function
> > .init.text:qcom_cpufreq_kryo_get_msm_id()
> > The function qcom_cpufreq_kryo_probe() references
> > the function __init qcom_cpufreq_kryo_get_msm_id().
> > This is often because qcom_cpufreq_kryo_probe lacks a __init
> > annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong.
> >
> > Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is
> > no more mismatch warning.
> 
> I wonder why this driver has an exit function marked __init rather
> than __exit?

I think it was just a mistake.

> Does that mean it gets cleaned up after kernel init, and
> so on unloading of the driver, the kernel jumps to unmapped memory?

The __init/exit sections are only useful when the driver is builtin
and so there is no unloading. Yeah, if you would have tried to call
shutdown for the kernel, it may have crashed or something. I don't
know.

> Does this patch now produce a warning for `qcom_cpufreq_kryo_driver`
> referencing `qcom_cpufreq_kryo_probe`?

Why should it ? It doesn't though.

-- 
viresh

Re: [PATCH v2] cpufreq: qcom-kryo: Fix section mismatch warning

[Viresh Kumar]

On 19-09-18, 14:50, Viresh Kumar wrote:
> On 19-09-18, 14:45, Nick Desaulniers wrote:
> > On Wed, Sep 19, 2018 at 11:22 AM Nathan Chancellor
> > <natechancellor@gmail.com> wrote:
> > >
> > > WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from
> > > the function qcom_cpufreq_kryo_probe() to the function
> > > .init.text:qcom_cpufreq_kryo_get_msm_id()
> > > The function qcom_cpufreq_kryo_probe() references
> > > the function __init qcom_cpufreq_kryo_get_msm_id().
> > > This is often because qcom_cpufreq_kryo_probe lacks a __init
> > > annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong.
> > >
> > > Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is
> > > no more mismatch warning.
> > 
> > I wonder why this driver has an exit function marked __init rather
> > than __exit?
> 
> I think it was just a mistake.
> 
> > Does that mean it gets cleaned up after kernel init, and
> > so on unloading of the driver, the kernel jumps to unmapped memory?
> 
> The __init/exit sections are only useful when the driver is builtin
> and so there is no unloading. Yeah, if you would have tried to call
> shutdown for the kernel, it may have crashed or something. I don't
> know.
> 
> > Does this patch now produce a warning for `qcom_cpufreq_kryo_driver`
> > referencing `qcom_cpufreq_kryo_probe`?
> 
> Why should it ? It doesn't though.

I thought you replied to my commit where I marked the exit routine
with __exit and realised just now that it wasn't the case. I haven't
build-tested this thing, but the question still stands. Why should it
?

-- 
viresh

Re: [PATCH v2] cpufreq: qcom-kryo: Fix section mismatch warning

[Nathan Chancellor]

On Wed, Sep 19, 2018 at 02:45:15PM -0700, Nick Desaulniers wrote:
> On Wed, Sep 19, 2018 at 11:22 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from
> > the function qcom_cpufreq_kryo_probe() to the function
> > .init.text:qcom_cpufreq_kryo_get_msm_id()
> > The function qcom_cpufreq_kryo_probe() references
> > the function __init qcom_cpufreq_kryo_get_msm_id().
> > This is often because qcom_cpufreq_kryo_probe lacks a __init
> > annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong.
> >
> > Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is
> > no more mismatch warning.
> 
> I wonder why this driver has an exit function marked __init rather
> than __exit? Does that mean it gets cleaned up after kernel init, and
> so on unloading of the driver, the kernel jumps to unmapped memory?
> 
> Does this patch now produce a warning for `qcom_cpufreq_kryo_driver`
> referencing `qcom_cpufreq_kryo_probe`?
> 

Hmmm I could have sworn I built this before sending it, although my
config might have been messed up but yes, I do see that warning.

WARNING: vmlinux.o(.data+0x117618): Section mismatch in reference from
the variable qcom_cpufreq_kryo_driver to the function
.init.text:qcom_cpufreq_kryo_probe()                                             
The variable qcom_cpufreq_kryo_driver references
the function __init qcom_cpufreq_kryo_probe() 

I suppose v1 might be the better option then as with the other patch,
marking qcom_cpufreq_kryo_driver doesn't seem correct.

I'm open to suggestions though.

Nathan

> >
> > Fixes: 46e2856b8e18 ("cpufreq: Add Kryo CPU scaling driver")
> > Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> >
> > v1 -> v2:
> >
> > Add '__init' to qcom_cpufreq_kryo_probe instead of removing it from
> > qcom_cpufreq_kryo_get_msm_id.
> >
> >  drivers/cpufreq/qcom-cpufreq-kryo.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/qcom-cpufreq-kryo.c b/drivers/cpufreq/qcom-cpufreq-kryo.c
> > index a1830fa25fc5..e001ef8ea7de 100644
> > --- a/drivers/cpufreq/qcom-cpufreq-kryo.c
> > +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c
> > @@ -73,7 +73,7 @@ static enum _msm8996_version __init qcom_cpufreq_kryo_get_msm_id(void)
> >         return version;
> >  }
> >
> > -static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
> > +static int __init qcom_cpufreq_kryo_probe(struct platform_device *pdev)
> >  {
> >         struct opp_table *opp_tables[NR_CPUS] = {0};
> >         enum _msm8996_version msm8996_version;
> > --
> > 2.19.0
> >
> 
> 
> --
> Thanks,
> ~Nick Desaulniers

Re: [PATCH v2] cpufreq: qcom-kryo: Fix section mismatch warning

[Rafael J. Wysocki]

On Wed, Sep 19, 2018 at 11:54 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 19-09-18, 14:50, Viresh Kumar wrote:
> > On 19-09-18, 14:45, Nick Desaulniers wrote:
> > > On Wed, Sep 19, 2018 at 11:22 AM Nathan Chancellor
> > > <natechancellor@gmail.com> wrote:
> > > >
> > > > WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from
> > > > the function qcom_cpufreq_kryo_probe() to the function
> > > > .init.text:qcom_cpufreq_kryo_get_msm_id()
> > > > The function qcom_cpufreq_kryo_probe() references
> > > > the function __init qcom_cpufreq_kryo_get_msm_id().
> > > > This is often because qcom_cpufreq_kryo_probe lacks a __init
> > > > annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong.
> > > >
> > > > Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is
> > > > no more mismatch warning.
> > >
> > > I wonder why this driver has an exit function marked __init rather
> > > than __exit?
> >
> > I think it was just a mistake.
> >
> > > Does that mean it gets cleaned up after kernel init, and
> > > so on unloading of the driver, the kernel jumps to unmapped memory?
> >
> > The __init/exit sections are only useful when the driver is builtin
> > and so there is no unloading. Yeah, if you would have tried to call
> > shutdown for the kernel, it may have crashed or something. I don't
> > know.
> >
> > > Does this patch now produce a warning for `qcom_cpufreq_kryo_driver`
> > > referencing `qcom_cpufreq_kryo_probe`?
> >
> > Why should it ? It doesn't though.
>
> I thought you replied to my commit where I marked the exit routine
> with __exit and realised just now that it wasn't the case. I haven't
> build-tested this thing, but the question still stands. Why should it
> ?

Because __init things go away at one point and calling them from the
other sections is a bad idea.  OTOH, __exit things are simply not
needed in built-in drivers and they are never there if the driver is
built-in, so calling them from the other sections is a bad idea too.

Can you guys, please, prepare *one* patch fixing all of the
__init/__exit annotations in this driver and post it?

Thanks,
Rafael

Re: [PATCH v2] cpufreq: qcom-kryo: Fix section mismatch warning

[Nathan Chancellor]

On Thu, Sep 20, 2018 at 12:45:55AM +0200, Rafael J. Wysocki wrote:
> On Wed, Sep 19, 2018 at 11:54 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 19-09-18, 14:50, Viresh Kumar wrote:
> > > On 19-09-18, 14:45, Nick Desaulniers wrote:
> > > > On Wed, Sep 19, 2018 at 11:22 AM Nathan Chancellor
> > > > <natechancellor@gmail.com> wrote:
> > > > >
> > > > > WARNING: vmlinux.o(.text+0x8aa424): Section mismatch in reference from
> > > > > the function qcom_cpufreq_kryo_probe() to the function
> > > > > .init.text:qcom_cpufreq_kryo_get_msm_id()
> > > > > The function qcom_cpufreq_kryo_probe() references
> > > > > the function __init qcom_cpufreq_kryo_get_msm_id().
> > > > > This is often because qcom_cpufreq_kryo_probe lacks a __init
> > > > > annotation or the annotation of qcom_cpufreq_kryo_get_msm_id is wrong.
> > > > >
> > > > > Add the '__init' annotation to qcom_cpufreq_kryo_probe so that there is
> > > > > no more mismatch warning.
> > > >
> > > > I wonder why this driver has an exit function marked __init rather
> > > > than __exit?
> > >
> > > I think it was just a mistake.
> > >
> > > > Does that mean it gets cleaned up after kernel init, and
> > > > so on unloading of the driver, the kernel jumps to unmapped memory?
> > >
> > > The __init/exit sections are only useful when the driver is builtin
> > > and so there is no unloading. Yeah, if you would have tried to call
> > > shutdown for the kernel, it may have crashed or something. I don't
> > > know.
> > >
> > > > Does this patch now produce a warning for `qcom_cpufreq_kryo_driver`
> > > > referencing `qcom_cpufreq_kryo_probe`?
> > >
> > > Why should it ? It doesn't though.
> >
> > I thought you replied to my commit where I marked the exit routine
> > with __exit and realised just now that it wasn't the case. I haven't
> > build-tested this thing, but the question still stands. Why should it
> > ?
> 
> Because __init things go away at one point and calling them from the
> other sections is a bad idea.  OTOH, __exit things are simply not
> needed in built-in drivers and they are never there if the driver is
> built-in, so calling them from the other sections is a bad idea too.
> 
> Can you guys, please, prepare *one* patch fixing all of the
> __init/__exit annotations in this driver and post it?
> 
> Thanks,
> Rafael

Hi Rafael,

Yes, I will work on this and send out a v3 for review.

Thanks,
Nathan

Re: [PATCH v2] cpufreq: qcom-kryo: Fix section mismatch warning

[Viresh Kumar]

On 20-09-18, 00:45, Rafael J. Wysocki wrote:
> Because __init things go away at one point and calling them from the
> other sections is a bad idea.  OTOH, __exit things are simply not
> needed in built-in drivers and they are never there if the driver is
> built-in, so calling them from the other sections is a bad idea too.

I assumed that the driver is calling platform_driver_probe() and so probe() can
be marked __init. But it doesn't do that to make deferred probing work.

-- 
viresh