Patchwork [1/1] apbuart: fix section mismatch warning

login
register
mail settings
Submitter Sam Ravnborg
Date Dec. 27, 2011, 9:22 p.m.
Message ID <20111227212253.GA8161@merkur.ravnborg.org>
Download mbox | patch
Permalink /patch/133366/
State Accepted
Delegated to: David Miller
Headers show

Comments

Sam Ravnborg - Dec. 27, 2011, 9:22 p.m.
From b82ccce349dee62fe5acd3167a19490bbe9a7e18 Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <sam@ravnborg.org>
Date: Tue, 27 Dec 2011 22:07:23 +0100
Subject: [PATCH] apbuart: fix section mismatch warning

Fix following warnings:

WARNING: drivers/tty/serial/built-in.o(.text+0x7370): Section mismatch in reference from the function grlib_apbuart_configure() to the variable .init.data:apbuart_match
The function grlib_apbuart_configure() references
the variable __initdata apbuart_match.
This is often because grlib_apbuart_configure lacks a __initdata
annotation or the annotation of apbuart_match is wrong.

+ 3 more warnings like this.

There is no guarantee that grlib_apbuart_of_driver.of_match_table
is only used at __init time - so drop the __initdata annotation.

grlib_apbuart_configure() is only used during __init so add __init
to this method too.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Daniel Hellstrom <daniel@gaisler.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/tty/serial/apbuart.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
David Miller - Dec. 27, 2011, 9:35 p.m.
From: Sam Ravnborg <sam@ravnborg.org>
Date: Tue, 27 Dec 2011 22:22:53 +0100

>>From b82ccce349dee62fe5acd3167a19490bbe9a7e18 Mon Sep 17 00:00:00 2001
> From: Sam Ravnborg <sam@ravnborg.org>
> Date: Tue, 27 Dec 2011 22:07:23 +0100
> Subject: [PATCH] apbuart: fix section mismatch warning
> 
> Fix following warnings:
> 
> WARNING: drivers/tty/serial/built-in.o(.text+0x7370): Section mismatch in reference from the function grlib_apbuart_configure() to the variable .init.data:apbuart_match
> The function grlib_apbuart_configure() references
> the variable __initdata apbuart_match.
> This is often because grlib_apbuart_configure lacks a __initdata
> annotation or the annotation of apbuart_match is wrong.
> 
> + 3 more warnings like this.
> 
> There is no guarantee that grlib_apbuart_of_driver.of_match_table
> is only used at __init time - so drop the __initdata annotation.
> 
> grlib_apbuart_configure() is only used during __init so add __init
> to this method too.
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Daniel Hellstrom <daniel@gaisler.com>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>

Applied to sparc-next, thanks.

Taking a look at the output of "git grep of_device_id | grep __initdata"
shows that a lot of drivers take liberties in this area, perhaps assuming
that they are built in or something like that.

But that isn't even a safe situation, because even built-in drivers can
have their of_device_id accessed later after we bootup, for example when
a bus is hot-plugged in or even via sysfs dumps of the avaialble drivers.

Probably the thing to do is to remove all of these __initdata tags,
because as far as I can tell they are _all_ wrong.  And perhaps add
'const' where missing as well.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sam Ravnborg - Dec. 27, 2011, 10:20 p.m.
On Tue, Dec 27, 2011 at 04:35:22PM -0500, David Miller wrote:
> From: Sam Ravnborg <sam@ravnborg.org>
> Date: Tue, 27 Dec 2011 22:22:53 +0100
> 
> >>>From b82ccce349dee62fe5acd3167a19490bbe9a7e18 Mon Sep 17 00:00:00 2001
> > From: Sam Ravnborg <sam@ravnborg.org>
> > Date: Tue, 27 Dec 2011 22:07:23 +0100
> > Subject: [PATCH] apbuart: fix section mismatch warning
> > 
> > Fix following warnings:
> > 
> > WARNING: drivers/tty/serial/built-in.o(.text+0x7370): Section mismatch in reference from the function grlib_apbuart_configure() to the variable .init.data:apbuart_match
> > The function grlib_apbuart_configure() references
> > the variable __initdata apbuart_match.
> > This is often because grlib_apbuart_configure lacks a __initdata
> > annotation or the annotation of apbuart_match is wrong.
> > 
> > + 3 more warnings like this.
> > 
> > There is no guarantee that grlib_apbuart_of_driver.of_match_table
> > is only used at __init time - so drop the __initdata annotation.
> > 
> > grlib_apbuart_configure() is only used during __init so add __init
> > to this method too.
> > 
> > Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Daniel Hellstrom <daniel@gaisler.com>
> > Cc: Greg Kroah-Hartman <gregkh@suse.de>
> 
> Applied to sparc-next, thanks.
> 
> Taking a look at the output of "git grep of_device_id | grep __initdata"
> shows that a lot of drivers take liberties in this area, perhaps assuming
> that they are built in or something like that.

I took a quick look at this.
In the places I found the of_device_id() was passed to for
example of_platform_bus_probe() which was only called at __init time.
And the function did not save the argument.

So at least some call sites are OK.
But I did not audit nearly all of them - so some bugs are lingering here.

	Sam
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Dec. 27, 2011, 10:28 p.m.
From: Sam Ravnborg <sam@ravnborg.org>
Date: Tue, 27 Dec 2011 23:20:46 +0100

> In the places I found the of_device_id() was passed to for
> example of_platform_bus_probe() which was only called at __init time.
> And the function did not save the argument.
> 
> So at least some call sites are OK.

Ok, good to know.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/tty/serial/apbuart.c b/drivers/tty/serial/apbuart.c
index 77554fd..7162f70 100644
--- a/drivers/tty/serial/apbuart.c
+++ b/drivers/tty/serial/apbuart.c
@@ -577,7 +577,7 @@  static int __devinit apbuart_probe(struct platform_device *op)
 	return 0;
 }
 
-static struct of_device_id __initdata apbuart_match[] = {
+static struct of_device_id apbuart_match[] = {
 	{
 	 .name = "GAISLER_APBUART",
 	 },
@@ -597,7 +597,7 @@  static struct platform_driver grlib_apbuart_of_driver = {
 };
 
 
-static int grlib_apbuart_configure(void)
+static int __init grlib_apbuart_configure(void)
 {
 	struct device_node *np;
 	int line = 0;