[PATCH] misc/cs5535: Fix section mismatch derived from cs5535_mfgpt_drv variable

[PATCH] misc/cs5535: Fix section mismatch derived from cs5535_mfgpt_drv variable

[Sedat Dilek]

>From my build.log:

WARNING: drivers/misc/cs5535-mfgpt.o(.data+0x0): Section mismatch in reference from the variable cs5535_mfgpt_drv to the function .devinit.text:cs5535_mfgpt_probe()
The variable cs5535_mfgpt_drv references
the function __devinit cs5535_mfgpt_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*driver, *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console,

This patch fixes the warning.

Tested with linux-next (next-20101231)

Signed-off-by: Sedat Dilek <sedat.dilek@gmail.com>
---
 drivers/misc/cs5535-mfgpt.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/misc/cs5535-mfgpt.c b/drivers/misc/cs5535-mfgpt.c
index d02d302..d80bd14 100644
--- a/drivers/misc/cs5535-mfgpt.c
+++ b/drivers/misc/cs5535-mfgpt.c
@@ -329,7 +329,7 @@ done:
 	return err;
 }
 
-static struct platform_driver cs5535_mfgpt_drv = {
+static struct platform_driver cs5535_mfgpt_drv __refdata = {
 	.driver = {
 		.name = DRV_NAME,
 		.owner = THIS_MODULE,
-- 
1.7.4.rc0

Re: [PATCH] misc/cs5535: Fix section mismatch derived from cs5535_mfgpt_drv variable

[Andres Salomon]

On Mon,  3 Jan 2011 03:51:28 +0100
Sedat Dilek <sedat.dilek@googlemail.com> wrote:

> From my build.log:
> 
> WARNING: drivers/misc/cs5535-mfgpt.o(.data+0x0): Section mismatch in
> reference from the variable cs5535_mfgpt_drv to the
> function .devinit.text:cs5535_mfgpt_probe() The variable
> cs5535_mfgpt_drv references the function __devinit
> cs5535_mfgpt_probe() If the reference is valid then annotate the
> variable with __init* or __refdata (see linux/init.h) or name the
> variable: *driver, *_template, *_timer, *_sht, *_ops, *_probe,
> *_probe_one, *_console,
> 
> This patch fixes the warning.
> 
> Tested with linux-next (next-20101231)
> 
> Signed-off-by: Sedat Dilek <sedat.dilek@gmail.com>
> ---
>  drivers/misc/cs5535-mfgpt.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/misc/cs5535-mfgpt.c b/drivers/misc/cs5535-mfgpt.c
> index d02d302..d80bd14 100644
> --- a/drivers/misc/cs5535-mfgpt.c
> +++ b/drivers/misc/cs5535-mfgpt.c
> @@ -329,7 +329,7 @@ done:
>  	return err;
>  }
>  
> -static struct platform_driver cs5535_mfgpt_drv = {
> +static struct platform_driver cs5535_mfgpt_drv __refdata = {
>  	.driver = {
>  		.name = DRV_NAME,
>  		.owner = THIS_MODULE,

Hm, I'm confused.  There are plenty of other drivers in mfd/ and gpio/
that have their probe/remove functions marked as __dev{init,exit}, with
their associated platform_driver definitions not marked with any kind
of init marking.  Are they all generating this warning and getting
the same __refdata treatment?

Re: [PATCH] misc/cs5535: Fix section mismatch derived from cs5535_mfgpt_drv variable

[Sedat Dilek]

[-- Attachment #1: Type: text/plain, Size: 2662 bytes --]

On Mon, Jan 3, 2011 at 10:52 AM, Andres Salomon <dilinger@queued.net> wrote:
> On Mon,  3 Jan 2011 03:51:28 +0100
> Sedat Dilek <sedat.dilek@googlemail.com> wrote:
>
>> From my build.log:
>>
>> WARNING: drivers/misc/cs5535-mfgpt.o(.data+0x0): Section mismatch in
>> reference from the variable cs5535_mfgpt_drv to the
>> function .devinit.text:cs5535_mfgpt_probe() The variable
>> cs5535_mfgpt_drv references the function __devinit
>> cs5535_mfgpt_probe() If the reference is valid then annotate the
>> variable with __init* or __refdata (see linux/init.h) or name the
>> variable: *driver, *_template, *_timer, *_sht, *_ops, *_probe,
>> *_probe_one, *_console,
>>
>> This patch fixes the warning.
>>
>> Tested with linux-next (next-20101231)
>>
>> Signed-off-by: Sedat Dilek <sedat.dilek@gmail.com>
>> ---
>>  drivers/misc/cs5535-mfgpt.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/misc/cs5535-mfgpt.c b/drivers/misc/cs5535-mfgpt.c
>> index d02d302..d80bd14 100644
>> --- a/drivers/misc/cs5535-mfgpt.c
>> +++ b/drivers/misc/cs5535-mfgpt.c
>> @@ -329,7 +329,7 @@ done:
>>       return err;
>>  }
>>
>> -static struct platform_driver cs5535_mfgpt_drv = {
>> +static struct platform_driver cs5535_mfgpt_drv __refdata = {
>>       .driver = {
>>               .name = DRV_NAME,
>>               .owner = THIS_MODULE,
>
> Hm, I'm confused.  There are plenty of other drivers in mfd/ and gpio/
> that have their probe/remove functions marked as __dev{init,exit}, with
> their associated platform_driver definitions not marked with any kind
> of init marking.  Are they all generating this warning and getting
> the same __refdata treatment?
>

Yes, there is also a section mismatch in "drivers/misc/ioc4.c" (see P.S.).
I hadn't time to look at it as it was very late 3 or 4 a.m. (and first
time sending patches via git-send-email :-)).

There are still some more section mismatches in other sub-trees, for
example "drivers/mtd/devices/sst25l.c" which I haven't catched.
If someone can look at it?

- Sedat -

P.S.: From my build_linux-next_next20101231.dileks.4.log:

WARNING: drivers/misc/ioc4.o(.data+0xc): Section mismatch in reference
from the variable ioc4_load_modules_work to the function
.devinit.text:ioc4_load_modules()
The variable ioc4_load_modules_work references
the function __devinit ioc4_load_modules()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*driver, *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console,

- EOT -

[-- Attachment #2: all-section-mismatches.txt --]
[-- Type: text/plain, Size: 4592 bytes --]

WARNING: drivers/video/geode/gx1fb.o(.data+0x54): Section mismatch in reference from the variable gx1fb_driver to the function .init.text:gx1fb_probe()
The variable gx1fb_driver references
the function __init gx1fb_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console, 

--
WARNING: drivers/misc/cs5535-mfgpt.o(.data+0x0): Section mismatch in reference from the variable cs5535_mfgpt_drv to the function .devinit.text:cs5535_mfgpt_probe()
The variable cs5535_mfgpt_drv references
the function __devinit cs5535_mfgpt_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*driver, *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console, 

WARNING: drivers/misc/ioc4.o(.data+0xc): Section mismatch in reference from the variable ioc4_load_modules_work to the function .devinit.text:ioc4_load_modules()
The variable ioc4_load_modules_work references
the function __devinit ioc4_load_modules()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*driver, *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console, 

WARNING: drivers/mtd/devices/sst25l.o(.devinit.text+0x7f): Section mismatch in reference from the function sst25l_match_device() to the variable .init.data:sst25l_flash_info
The function __devinit sst25l_match_device() references
a variable __initdata sst25l_flash_info.
If sst25l_flash_info is only used by sst25l_match_device then
annotate sst25l_flash_info with a matching annotation.

WARNING: drivers/mtd/devices/sst25l.o(.devinit.text+0x94): Section mismatch in reference from the function sst25l_match_device() to the variable .init.data:sst25l_flash_info
The function __devinit sst25l_match_device() references
a variable __initdata sst25l_flash_info.
If sst25l_flash_info is only used by sst25l_match_device then
annotate sst25l_flash_info with a matching annotation.

WARNING: drivers/mtd/devices/sst25l.o(.devinit.text+0x9e): Section mismatch in reference from the function sst25l_match_device() to the (unknown reference) .init.data:(unknown)
The function __devinit sst25l_match_device() references
a (unknown reference) __initdata (unknown).
If (unknown) is only used by sst25l_match_device then
annotate (unknown) with a matching annotation.

WARNING: drivers/mtd/devices/sst25l.o(.devinit.text+0xa5): Section mismatch in reference from the function sst25l_match_device() to the (unknown reference) .init.data:(unknown)
The function __devinit sst25l_match_device() references
a (unknown reference) __initdata (unknown).
If (unknown) is only used by sst25l_match_device then
annotate (unknown) with a matching annotation.

  MKPIGGY arch/x86/boot/compressed/piggy.S
--
WARNING: drivers/net/depca.o(.data+0x0): Section mismatch in reference from the variable depca_isa_driver to the function .init.text:depca_isa_probe()
The variable depca_isa_driver references
the function __init depca_isa_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console, 

--
drivers/net/irda/smsc-ircc2.o(.data+0x18): Section mismatch in reference from the variable smsc_ircc_pnp_driver to the function .init.text:smsc_ircc_pnp_probe()
The variable smsc_ircc_pnp_driver references
the function __init smsc_ircc_pnp_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console, 

--
WARNING: drivers/net/ksz884x.o(.data+0x18): Section mismatch in reference from the variable pci_device_driver to the function .init.text:pcidev_init()
The variable pci_device_driver references
the function __init pcidev_init()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console, 

WARNING: drivers/video/geode/gx1fb.o(.data+0x54): Section mismatch in reference from the variable gx1fb_driver to the function .init.text:gx1fb_probe()
The variable gx1fb_driver references
the function __init gx1fb_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console, 

Re: [PATCH] misc/cs5535: Fix section mismatch derived from cs5535_mfgpt_drv variable

[Andrew Morton]

On Mon,  3 Jan 2011 03:51:28 +0100
Sedat Dilek <sedat.dilek@googlemail.com> wrote:

> >From my build.log:
> 
> WARNING: drivers/misc/cs5535-mfgpt.o(.data+0x0): Section mismatch in reference from the variable cs5535_mfgpt_drv to the function .devinit.text:cs5535_mfgpt_probe()
> The variable cs5535_mfgpt_drv references
> the function __devinit cs5535_mfgpt_probe()
> If the reference is valid then annotate the
> variable with __init* or __refdata (see linux/init.h) or name the variable:
> *driver, *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console,

Confused.  cs5535_mfgpt_probe() _does_ have a name of the form
"*_probe", so why did it warn?

Re: [PATCH] misc/cs5535: Fix section mismatch derived from cs5535_mfgpt_drv variable

[Sam Ravnborg]

On Mon, Jan 03, 2011 at 11:43:10AM -0800, Andrew Morton wrote:
> On Mon,  3 Jan 2011 03:51:28 +0100
> Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> 
> > >From my build.log:
> > 
> > WARNING: drivers/misc/cs5535-mfgpt.o(.data+0x0): Section mismatch in reference from the variable cs5535_mfgpt_drv to the function .devinit.text:cs5535_mfgpt_probe()
> > The variable cs5535_mfgpt_drv references
> > the function __devinit cs5535_mfgpt_probe()
> > If the reference is valid then annotate the
> > variable with __init* or __refdata (see linux/init.h) or name the variable:
> > *driver, *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console,
> 
> Confused.  cs5535_mfgpt_probe() _does_ have a name of the form
> "*_probe", so why did it warn?

The varibale need to follow the naming scheme.
In this case the function was named *_probe - which has no significance
in the section mismatch checks.

> > variable with __init* or __refdata (see linux/init.h) or name the variable:
    ^^^^^^^^                                                          ^^^^^^^^^ 

	Sam

Re: [PATCH] misc/cs5535: Fix section mismatch derived from cs5535_mfgpt_drv variable

[Andres Salomon]

On Mon, 3 Jan 2011 20:51:31 +0100
Sam Ravnborg <sam@ravnborg.org> wrote:

> On Mon, Jan 03, 2011 at 11:43:10AM -0800, Andrew Morton wrote:
> > On Mon,  3 Jan 2011 03:51:28 +0100
> > Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> > 
> > > >From my build.log:
> > > 
> > > WARNING: drivers/misc/cs5535-mfgpt.o(.data+0x0): Section mismatch
> > > in reference from the variable cs5535_mfgpt_drv to the
> > > function .devinit.text:cs5535_mfgpt_probe() The variable
> > > cs5535_mfgpt_drv references the function __devinit
> > > cs5535_mfgpt_probe() If the reference is valid then annotate the
> > > variable with __init* or __refdata (see linux/init.h) or name the
> > > variable: *driver, *_template, *_timer, *_sht, *_ops, *_probe,
> > > *_probe_one, *_console,
> > 
> > Confused.  cs5535_mfgpt_probe() _does_ have a name of the form
> > "*_probe", so why did it warn?
> 
> The varibale need to follow the naming scheme.
> In this case the function was named *_probe - which has no
> significance in the section mismatch checks.
> 
> > > variable with __init* or __refdata (see linux/init.h) or name the
> > > variable:
>     ^^^^^^^^
> ^^^^^^^^^ 
> 
> 	Sam

I see.  So it should really be renamed cs5535_mfgpt_driver, or _drv
added to the whitelist.  Or the whitelist dropped, and all drivers
changed to explicitly mark the structs with __devinitdata.

It's kind of bizarre that we have variable names signifying whether or
not warnings get issued..

Re: [PATCH] misc/cs5535: Fix section mismatch derived from cs5535_mfgpt_drv variable

[Sam Ravnborg]

On Mon, Jan 03, 2011 at 12:34:31PM -0800, Andres Salomon wrote:
> On Mon, 3 Jan 2011 20:51:31 +0100
> Sam Ravnborg <sam@ravnborg.org> wrote:
> 
> > On Mon, Jan 03, 2011 at 11:43:10AM -0800, Andrew Morton wrote:
> > > On Mon,  3 Jan 2011 03:51:28 +0100
> > > Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> > > 
> > > > >From my build.log:
> > > > 
> > > > WARNING: drivers/misc/cs5535-mfgpt.o(.data+0x0): Section mismatch
> > > > in reference from the variable cs5535_mfgpt_drv to the
> > > > function .devinit.text:cs5535_mfgpt_probe() The variable
> > > > cs5535_mfgpt_drv references the function __devinit
> > > > cs5535_mfgpt_probe() If the reference is valid then annotate the
> > > > variable with __init* or __refdata (see linux/init.h) or name the
> > > > variable: *driver, *_template, *_timer, *_sht, *_ops, *_probe,
> > > > *_probe_one, *_console,
> > > 
> > > Confused.  cs5535_mfgpt_probe() _does_ have a name of the form
> > > "*_probe", so why did it warn?
> > 
> > The varibale need to follow the naming scheme.
> > In this case the function was named *_probe - which has no
> > significance in the section mismatch checks.
> > 
> > > > variable with __init* or __refdata (see linux/init.h) or name the
> > > > variable:
> >     ^^^^^^^^
> > ^^^^^^^^^ 
> > 
> > 	Sam
> 
> I see.  So it should really be renamed cs5535_mfgpt_driver, or _drv
> added to the whitelist.  Or the whitelist dropped, and all drivers
> changed to explicitly mark the structs with __devinitdata.

It is preferred that the variables are annotated __refdata.
> 
> It's kind of bizarre that we have variable names signifying whether or
> not warnings get issued..

It really helped to remove a lot of nosie when we introduced this check.

	Sam

Re: [PATCH] misc/cs5535: Fix section mismatch derived from cs5535_mfgpt_drv variable

[Andres Salomon]

On Mon, 3 Jan 2011 21:50:19 +0100
Sam Ravnborg <sam@ravnborg.org> wrote:

> On Mon, Jan 03, 2011 at 12:34:31PM -0800, Andres Salomon wrote:
> > On Mon, 3 Jan 2011 20:51:31 +0100
> > Sam Ravnborg <sam@ravnborg.org> wrote:
> > 
> > > On Mon, Jan 03, 2011 at 11:43:10AM -0800, Andrew Morton wrote:
> > > > On Mon,  3 Jan 2011 03:51:28 +0100
> > > > Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> > > > 
> > > > > >From my build.log:
> > > > > 
> > > > > WARNING: drivers/misc/cs5535-mfgpt.o(.data+0x0): Section
> > > > > mismatch in reference from the variable cs5535_mfgpt_drv to
> > > > > the function .devinit.text:cs5535_mfgpt_probe() The variable
> > > > > cs5535_mfgpt_drv references the function __devinit
> > > > > cs5535_mfgpt_probe() If the reference is valid then annotate
> > > > > the variable with __init* or __refdata (see linux/init.h) or
> > > > > name the variable: *driver, *_template, *_timer, *_sht,
> > > > > *_ops, *_probe, *_probe_one, *_console,
> > > > 
> > > > Confused.  cs5535_mfgpt_probe() _does_ have a name of the form
> > > > "*_probe", so why did it warn?
> > > 
> > > The varibale need to follow the naming scheme.
> > > In this case the function was named *_probe - which has no
> > > significance in the section mismatch checks.
> > > 
> > > > > variable with __init* or __refdata (see linux/init.h) or name
> > > > > the variable:
> > >     ^^^^^^^^
> > > ^^^^^^^^^ 
> > > 
> > > 	Sam
> > 
> > I see.  So it should really be renamed cs5535_mfgpt_driver, or _drv
> > added to the whitelist.  Or the whitelist dropped, and all drivers
> > changed to explicitly mark the structs with __devinitdata.
> 
> It is preferred that the variables are annotated __refdata.

Should they not be devinitdata, since they're structures referencing
devinit (ie, hotplug init) hooks?

The two places where I traditionally went for documentation on such
things was linux/init.h and Documentation/PCI/pci.txt.  The latter
states:

        __devinit       Device initialization code.
                        Identical to __init if the kernel is not
        compiled with CONFIG_HOTPLUG, normal function otherwise.

        o Do not mark the struct pci_driver.

linux/init.h says:

 * modpost not to issue a warning.  Intended semantics is that a code or
 * data tagged __ref* can reference code or data from init section
   without
 * producing a warning (of course, no warning does not mean code is
 * correct, so optimally document why the __ref is needed and why it's
   OK).


It would be nice to know why refdata is correct here, and devinitdata
is not.

Re: [PATCH] misc/cs5535: Fix section mismatch derived from cs5535_mfgpt_drv variable

[Sam Ravnborg]

On Mon, Jan 03, 2011 at 01:14:03PM -0800, Andres Salomon wrote:
> On Mon, 3 Jan 2011 21:50:19 +0100
> Sam Ravnborg <sam@ravnborg.org> wrote:
> 
> > On Mon, Jan 03, 2011 at 12:34:31PM -0800, Andres Salomon wrote:
> > > On Mon, 3 Jan 2011 20:51:31 +0100
> > > Sam Ravnborg <sam@ravnborg.org> wrote:
> > > 
> > > > On Mon, Jan 03, 2011 at 11:43:10AM -0800, Andrew Morton wrote:
> > > > > On Mon,  3 Jan 2011 03:51:28 +0100
> > > > > Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> > > > > 
> > > > > > >From my build.log:
> > > > > > 
> > > > > > WARNING: drivers/misc/cs5535-mfgpt.o(.data+0x0): Section
> > > > > > mismatch in reference from the variable cs5535_mfgpt_drv to
> > > > > > the function .devinit.text:cs5535_mfgpt_probe() The variable
> > > > > > cs5535_mfgpt_drv references the function __devinit
> > > > > > cs5535_mfgpt_probe() If the reference is valid then annotate
> > > > > > the variable with __init* or __refdata (see linux/init.h) or
> > > > > > name the variable: *driver, *_template, *_timer, *_sht,
> > > > > > *_ops, *_probe, *_probe_one, *_console,
> > > > > 
> > > > > Confused.  cs5535_mfgpt_probe() _does_ have a name of the form
> > > > > "*_probe", so why did it warn?
> > > > 
> > > > The varibale need to follow the naming scheme.
> > > > In this case the function was named *_probe - which has no
> > > > significance in the section mismatch checks.
> > > > 
> > > > > > variable with __init* or __refdata (see linux/init.h) or name
> > > > > > the variable:
> > > >     ^^^^^^^^
> > > > ^^^^^^^^^ 
> > > > 
> > > > 	Sam
> > > 
> > > I see.  So it should really be renamed cs5535_mfgpt_driver, or _drv
> > > added to the whitelist.  Or the whitelist dropped, and all drivers
> > > changed to explicitly mark the structs with __devinitdata.
> > 
> > It is preferred that the variables are annotated __refdata.
> 
> Should they not be devinitdata, since they're structures referencing
> devinit (ie, hotplug init) hooks?
> 
> The two places where I traditionally went for documentation on such
> things was linux/init.h and Documentation/PCI/pci.txt.  The latter
> states:
> 
>         __devinit       Device initialization code.
>                         Identical to __init if the kernel is not
>         compiled with CONFIG_HOTPLUG, normal function otherwise.
> 
>         o Do not mark the struct pci_driver.
> 
> linux/init.h says:
> 
>  * modpost not to issue a warning.  Intended semantics is that a code or
>  * data tagged __ref* can reference code or data from init section
>    without
>  * producing a warning (of course, no warning does not mean code is
>  * correct, so optimally document why the __ref is needed and why it's
>    OK).
> 
> 
> It would be nice to know why refdata is correct here, and devinitdata
> is not.

The annotation really depends on the lifetime of the variable.

A variable that is used when the kernel is fully operational shall be
annotated with __refdata.
This will relocate the data to a special section that modpost
recognize and thus do not warn about references to functions
/data that are discarded during the startup phase.

__init is used for data that are uncnditionally discarded after
the init phase.
__devinitdata is used for data that is discarded after the initphase
is hotplug is not enabled - if hotplug is enabled then the
data are not discared.

So __init and __initdata have impact on the lifetime of the
data  + how modpost check references - where __refdata is
only used to shut up warnings from modpost.

__refdata was introduced late in the process where we
fixed a lot of section mismatch warnings.
So most of the fixes that hit the kernel was renaming
variables so references to init data/functions did not
cause warnings.

I looked into a more precise way to do the checkss
back then so we could do the check down on member
level in for example a "struct driver" - but I never
came up with anything good.
So therefore we are stuck with the less optimal check
algorithm today.

	Sam