Patchwork powerpc/legacy_serial: fix incorrect placement of __initdata tag

login
register
mail settings
Submitter Bartlomiej Zolnierkiewicz
Date Sept. 30, 2013, 1:11 p.m.
Message ID <35419229.o6Cv8JYbpg@amdc1032>
Download mbox | patch
Permalink /patch/279150/
State Accepted
Commit dc1473dcfaf4fb38ba2dcb2fb7ac7d0242185fa3
Delegated to: Benjamin Herrenschmidt
Headers show

Comments

Bartlomiej Zolnierkiewicz - Sept. 30, 2013, 1:11 p.m.
__initdata tag should be placed between the variable name and equal
sign for the variable to be placed in the intended .init.data section.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/powerpc/kernel/legacy_serial.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Michael Ellerman - Oct. 1, 2013, 6:13 a.m.
On Mon, Sep 30, 2013 at 03:11:42PM +0200, Bartlomiej Zolnierkiewicz wrote:
> __initdata tag should be placed between the variable name and equal
> sign for the variable to be placed in the intended .init.data section.

I see lots of other occurences of that in arch/powerpc. Why not send a
single patch to update them all?

cheers
Bartlomiej Zolnierkiewicz - Oct. 3, 2013, 11:51 a.m.
On Tuesday, October 01, 2013 04:13:25 PM Michael Ellerman wrote:
> On Mon, Sep 30, 2013 at 03:11:42PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > __initdata tag should be placed between the variable name and equal
> > sign for the variable to be placed in the intended .init.data section.
> 
> I see lots of other occurences of that in arch/powerpc. Why not send a
> single patch to update them all?

The other occurences while not following the preferred kernel coding style
are (probably) working OK with gcc. This particular occurence just doesn't
work as it should.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Michael Ellerman - Oct. 8, 2013, 3:56 a.m.
On Thu, Oct 03, 2013 at 01:51:27PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Tuesday, October 01, 2013 04:13:25 PM Michael Ellerman wrote:
> > On Mon, Sep 30, 2013 at 03:11:42PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > __initdata tag should be placed between the variable name and equal
> > > sign for the variable to be placed in the intended .init.data section.
> > 
> > I see lots of other occurences of that in arch/powerpc. Why not send a
> > single patch to update them all?
> 
> The other occurences while not following the preferred kernel coding style
> are (probably) working OK with gcc. This particular occurence just doesn't
> work as it should.

Why would the other occurrences work but not this one?

Regardless, why don't we just do a single patch to clean them all up to
match coding style and (probably) do what they're intended.

cheers
Bartlomiej Zolnierkiewicz - Oct. 8, 2013, 9:33 a.m.
On Tuesday, October 08, 2013 02:56:23 PM Michael Ellerman wrote:
> On Thu, Oct 03, 2013 at 01:51:27PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Tuesday, October 01, 2013 04:13:25 PM Michael Ellerman wrote:
> > > On Mon, Sep 30, 2013 at 03:11:42PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > > __initdata tag should be placed between the variable name and equal
> > > > sign for the variable to be placed in the intended .init.data section.
> > > 
> > > I see lots of other occurences of that in arch/powerpc. Why not send a
> > > single patch to update them all?
> > 
> > The other occurences while not following the preferred kernel coding style
> > are (probably) working OK with gcc. This particular occurence just doesn't
> > work as it should.
> 
> Why would the other occurrences work but not this one?

Because gcc seems to generate the correct code for things like i.e. this one:

struct of_device_id __initdata legacy_serial_parents[]

but not for ones like this:

struct __initdata of_device_id legacy_serial_parents[]

> Regardless, why don't we just do a single patch to clean them all up to
> match coding style and (probably) do what they're intended.

Because:

- fixing this occurence changes runtime, fixing others won't

- there were no such request from powerpc Maintainers

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Patch

diff --git a/arch/powerpc/kernel/legacy_serial.c b/arch/powerpc/kernel/legacy_serial.c
index 22e88dd..40bd7bd 100644
--- a/arch/powerpc/kernel/legacy_serial.c
+++ b/arch/powerpc/kernel/legacy_serial.c
@@ -35,7 +35,7 @@  static struct legacy_serial_info {
 	phys_addr_t			taddr;
 } legacy_serial_infos[MAX_LEGACY_SERIAL_PORTS];
 
-static struct __initdata of_device_id legacy_serial_parents[] = {
+static struct of_device_id legacy_serial_parents[] __initdata = {
 	{.type = "soc",},
 	{.type = "tsi-bridge",},
 	{.type = "opb", },