* [PATCH] ARM: tegra: paz00: fix __initdata placement
@ 2017-01-23 7:43 Dmitry Torokhov
2017-01-23 10:04 ` Marc Dietrich
2017-01-25 8:11 ` Thierry Reding
0 siblings, 2 replies; 6+ messages in thread
From: Dmitry Torokhov @ 2017-01-23 7:43 UTC (permalink / raw)
To: Stephen Warren, Thierry Reding, Alexandre Courbot
Cc: Heikki Krogerus, linux-arm-kernel, linux-tegra, linux-kernel
To have expected effect the __initdata attribute should go after variable
name and before initializer.`
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
arch/arm/mach-tegra/board-paz00.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
index 7478f6fb3664..ea6bff404161 100644
--- a/arch/arm/mach-tegra/board-paz00.c
+++ b/arch/arm/mach-tegra/board-paz00.c
@@ -23,7 +23,7 @@
#include "board.h"
-static struct property_entry __initdata wifi_rfkill_prop[] = {
+static struct property_entry wifi_rfkill_prop[] __initdata = {
PROPERTY_ENTRY_STRING("name", "wifi_rfkill"),
PROPERTY_ENTRY_STRING("type", "wlan"),
{ },
--
2.11.0.483.g087da7b7c-goog
--
Dmitry
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: tegra: paz00: fix __initdata placement
2017-01-23 7:43 [PATCH] ARM: tegra: paz00: fix __initdata placement Dmitry Torokhov
@ 2017-01-23 10:04 ` Marc Dietrich
2017-01-23 16:24 ` Thierry Reding
2017-01-25 8:11 ` Thierry Reding
1 sibling, 1 reply; 6+ messages in thread
From: Marc Dietrich @ 2017-01-23 10:04 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Stephen Warren, Thierry Reding, Alexandre Courbot,
Heikki Krogerus, linux-arm-kernel, linux-tegra, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]
Hello Dmitry,
Am Sonntag, 22. Januar 2017, 23:43:47 CET schrieb Dmitry Torokhov:
> To have expected effect the __initdata attribute should go after variable
> name and before initializer.`
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> arch/arm/mach-tegra/board-paz00.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-tegra/board-paz00.c
> b/arch/arm/mach-tegra/board-paz00.c index 7478f6fb3664..ea6bff404161 100644
> --- a/arch/arm/mach-tegra/board-paz00.c
> +++ b/arch/arm/mach-tegra/board-paz00.c
> @@ -23,7 +23,7 @@
>
> #include "board.h"
>
> -static struct property_entry __initdata wifi_rfkill_prop[] = {
> +static struct property_entry wifi_rfkill_prop[] __initdata = {
> PROPERTY_ENTRY_STRING("name", "wifi_rfkill"),
> PROPERTY_ENTRY_STRING("type", "wlan"),
> { },
you are right according to the documentation, but objdump -s always shows that
it gets put into the .rodata section. So this patch has no effect, because
result is always same (and wrong). It's also possible that I'm doing something
wrong :-) Btw, there are hundreds of such __initdata misplacement in the
kernel.
Marc
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: tegra: paz00: fix __initdata placement
2017-01-23 10:04 ` Marc Dietrich
@ 2017-01-23 16:24 ` Thierry Reding
2017-01-24 9:08 ` Marc Dietrich
2017-01-24 18:27 ` Dmitry Torokhov
0 siblings, 2 replies; 6+ messages in thread
From: Thierry Reding @ 2017-01-23 16:24 UTC (permalink / raw)
To: Marc Dietrich
Cc: Dmitry Torokhov, Stephen Warren, Alexandre Courbot,
Heikki Krogerus, linux-arm-kernel, linux-tegra, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3857 bytes --]
On Mon, Jan 23, 2017 at 11:04:22AM +0100, Marc Dietrich wrote:
> Hello Dmitry,
>
> Am Sonntag, 22. Januar 2017, 23:43:47 CET schrieb Dmitry Torokhov:
> > To have expected effect the __initdata attribute should go after variable
> > name and before initializer.`
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > arch/arm/mach-tegra/board-paz00.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-tegra/board-paz00.c
> > b/arch/arm/mach-tegra/board-paz00.c index 7478f6fb3664..ea6bff404161 100644
> > --- a/arch/arm/mach-tegra/board-paz00.c
> > +++ b/arch/arm/mach-tegra/board-paz00.c
> > @@ -23,7 +23,7 @@
> >
> > #include "board.h"
> >
> > -static struct property_entry __initdata wifi_rfkill_prop[] = {
> > +static struct property_entry wifi_rfkill_prop[] __initdata = {
> > PROPERTY_ENTRY_STRING("name", "wifi_rfkill"),
> > PROPERTY_ENTRY_STRING("type", "wlan"),
> > { },
>
> you are right according to the documentation, but objdump -s always shows that
> it gets put into the .rodata section. So this patch has no effect, because
> result is always same (and wrong). It's also possible that I'm doing something
> wrong :-) Btw, there are hundreds of such __initdata misplacement in the
> kernel.
Hmm... I get this:
$ objdump -t build/tegra20/arch/arm/mach-tegra/board-paz00.o
build/tegra20/arch/arm/mach-tegra/board-paz00.o: file format
elf32-little
SYMBOL TABLE:
00000000 l df *ABS* 00000000 board-paz00.c
00000000 l d .text 00000000 .text
00000000 l d .data 00000000 .data
00000000 l d .bss 00000000 .bss
00000000 l d .init.text 00000000 .init.text
00000000 l .init.text 00000000 $a
00000000 l .data 00000000 .LANCHOR1
00000000 l .init.data 00000000 .LANCHOR0
00000000 l d .ARM.extab.init.text 00000000 .ARM.extab.init.text
00000000 l d .ARM.exidx.init.text 00000000 .ARM.exidx.init.text
00000000 l .ARM.exidx.init.text 00000000 $d
00000000 l .data 00000000 $d
00000000 l O .data 000001c8 wifi_rfkill_device
000001c8 l O .data 00000048 wifi_gpio_lookup
00000000 l d .rodata.str1.4 00000000 .rodata.str1.4
00000000 l .rodata.str1.4 00000000 $d
00000000 l d .init.data 00000000 .init.data
00000000 l .init.data 00000000 $d
00000000 l O .init.data 00000048 wifi_rfkill_prop
00000000 l d .debug_frame 00000000 .debug_frame
00000010 l .debug_frame 00000000 $d
00000000 l d .debug_info 00000000 .debug_info
00000000 l d .debug_abbrev 00000000 .debug_abbrev
00000000 l d .debug_aranges 00000000 .debug_aranges
00000000 l d .debug_ranges 00000000 .debug_ranges
00000000 l d .debug_line 00000000 .debug_line
00000000 l d .debug_str 00000000 .debug_str
00000000 l d .note.GNU-stack 00000000 .note.GNU-stack
00000000 l d .comment 00000000 .comment
00000000 l d .ARM.attributes 00000000 .ARM.attributes
00000000 g F .init.text 00000030 tegra_paz00_wifikill_init
00000000 *UND* 00000000 platform_device_add_properties
00000000 *UND* 00000000 gpiod_add_lookup_table
00000000 *UND* 00000000 platform_device_register
00000000 *UND* 00000000 __aeabi_unwind_cpp_pr0
So it correctly ends up in .init.data for me. And it does so with or
without the patch. GCCs documentation doesn't seem to say where exactly
these attributes need to be put, though the examples given all have the
attribute right before the =.
Oh... interestingly checkpatch does seem to have a check for this now,
and it does recommend that __initdata be placed after wifi_rfkill_prop,
so I'm going to apply this.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: tegra: paz00: fix __initdata placement
2017-01-23 16:24 ` Thierry Reding
@ 2017-01-24 9:08 ` Marc Dietrich
2017-01-24 18:27 ` Dmitry Torokhov
1 sibling, 0 replies; 6+ messages in thread
From: Marc Dietrich @ 2017-01-24 9:08 UTC (permalink / raw)
To: Thierry Reding
Cc: Dmitry Torokhov, Stephen Warren, Alexandre Courbot,
Heikki Krogerus, linux-arm-kernel, linux-tegra, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5091 bytes --]
Hello Thierry,
Am Montag, 23. Januar 2017, 17:24:28 CET schrieb Thierry Reding:
> On Mon, Jan 23, 2017 at 11:04:22AM +0100, Marc Dietrich wrote:
> > Hello Dmitry,
> >
> > Am Sonntag, 22. Januar 2017, 23:43:47 CET schrieb Dmitry Torokhov:
> > > To have expected effect the __initdata attribute should go after
> > > variable
> > > name and before initializer.`
> > >
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > ---
> > >
> > > arch/arm/mach-tegra/board-paz00.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/mach-tegra/board-paz00.c
> > > b/arch/arm/mach-tegra/board-paz00.c index 7478f6fb3664..ea6bff404161
> > > 100644
> > > --- a/arch/arm/mach-tegra/board-paz00.c
> > > +++ b/arch/arm/mach-tegra/board-paz00.c
> > > @@ -23,7 +23,7 @@
> > >
> > > #include "board.h"
> > >
> > > -static struct property_entry __initdata wifi_rfkill_prop[] = {
> > > +static struct property_entry wifi_rfkill_prop[] __initdata = {
> > >
> > > PROPERTY_ENTRY_STRING("name", "wifi_rfkill"),
> > > PROPERTY_ENTRY_STRING("type", "wlan"),
> > > { },
> >
> > you are right according to the documentation, but objdump -s always shows
> > that it gets put into the .rodata section. So this patch has no effect,
> > because result is always same (and wrong). It's also possible that I'm
> > doing something wrong :-) Btw, there are hundreds of such __initdata
> > misplacement in the kernel.
>
> Hmm... I get this:
>
> $ objdump -t build/tegra20/arch/arm/mach-tegra/board-paz00.o
>
> build/tegra20/arch/arm/mach-tegra/board-paz00.o: file format
> elf32-little
>
> SYMBOL TABLE:
> 00000000 l df *ABS* 00000000 board-paz00.c
> 00000000 l d .text 00000000 .text
> 00000000 l d .data 00000000 .data
> 00000000 l d .bss 00000000 .bss
> 00000000 l d .init.text 00000000 .init.text
> 00000000 l .init.text 00000000 $a
> 00000000 l .data 00000000 .LANCHOR1
> 00000000 l .init.data 00000000 .LANCHOR0
> 00000000 l d .ARM.extab.init.text 00000000 .ARM.extab.init.text
> 00000000 l d .ARM.exidx.init.text 00000000 .ARM.exidx.init.text
> 00000000 l .ARM.exidx.init.text 00000000 $d
> 00000000 l .data 00000000 $d
> 00000000 l O .data 000001c8 wifi_rfkill_device
> 000001c8 l O .data 00000048 wifi_gpio_lookup
> 00000000 l d .rodata.str1.4 00000000 .rodata.str1.4
> 00000000 l .rodata.str1.4 00000000 $d
> 00000000 l d .init.data 00000000 .init.data
> 00000000 l .init.data 00000000 $d
> 00000000 l O .init.data 00000048 wifi_rfkill_prop
> 00000000 l d .debug_frame 00000000 .debug_frame
> 00000010 l .debug_frame 00000000 $d
> 00000000 l d .debug_info 00000000 .debug_info
> 00000000 l d .debug_abbrev 00000000 .debug_abbrev
> 00000000 l d .debug_aranges 00000000 .debug_aranges
> 00000000 l d .debug_ranges 00000000 .debug_ranges
> 00000000 l d .debug_line 00000000 .debug_line
> 00000000 l d .debug_str 00000000 .debug_str
> 00000000 l d .note.GNU-stack 00000000 .note.GNU-stack
> 00000000 l d .comment 00000000 .comment
> 00000000 l d .ARM.attributes 00000000 .ARM.attributes
> 00000000 g F .init.text 00000030 tegra_paz00_wifikill_init
> 00000000 *UND* 00000000 platform_device_add_properties
> 00000000 *UND* 00000000 gpiod_add_lookup_table
> 00000000 *UND* 00000000 platform_device_register
> 00000000 *UND* 00000000 __aeabi_unwind_cpp_pr0
>
> So it correctly ends up in .init.data for me. And it does so with or
> without the patch. GCCs documentation doesn't seem to say where exactly
> these attributes need to be put, though the examples given all have the
> attribute right before the =.
ok, objdump -s is maybe not appropriate to display such things.
Contents of section .init.data:
0000 2c000000 0c000000 00010000 00000000 ,...............
0010 34000000 00000000 40000000 05000000 4.......@.......
0020 00010000 00000000 48000000 00000000 ........H.......
0030 00000000 00000000 00000000 00000000 ................
0040 00000000 00000000 ........
Contents of section .rodata.str1.4:
0000 72666b69 6c6c5f67 70696f00 74656772 rfkill_gpio.tegr
0010 612d6770 696f0000 72657365 74000000 a-gpio..reset...
0020 73687574 646f776e 00000000 6e616d65 shutdown....name
0030 00000000 77696669 5f72666b 696c6c00 ....wifi_rfkill.
0040 74797065 00000000 776c616e 00 type....wlan.
In fact, .init.data seems to contain relocations to .rodata.str1.4, which "-s"
does not resolve.
> Oh... interestingly checkpatch does seem to have a check for this now,
> and it does recommend that __initdata be placed after wifi_rfkill_prop,
> so I'm going to apply this.
Btw, where is your tree for 4.11? git.kernel.org shows no action since
November. Please also include https://patchwork.ozlabs.org/patch/704367/ if
you haven't done yet. I forgot to add a #stable tag for 4.9 and 4.10.
Marc
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: tegra: paz00: fix __initdata placement
2017-01-23 16:24 ` Thierry Reding
2017-01-24 9:08 ` Marc Dietrich
@ 2017-01-24 18:27 ` Dmitry Torokhov
1 sibling, 0 replies; 6+ messages in thread
From: Dmitry Torokhov @ 2017-01-24 18:27 UTC (permalink / raw)
To: Thierry Reding
Cc: Marc Dietrich, Stephen Warren, Alexandre Courbot,
Heikki Krogerus, linux-arm-kernel, linux-tegra, linux-kernel
On Mon, Jan 23, 2017 at 05:24:28PM +0100, Thierry Reding wrote:
> On Mon, Jan 23, 2017 at 11:04:22AM +0100, Marc Dietrich wrote:
> > Hello Dmitry,
> >
> > Am Sonntag, 22. Januar 2017, 23:43:47 CET schrieb Dmitry Torokhov:
> > > To have expected effect the __initdata attribute should go after variable
> > > name and before initializer.`
> > >
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > ---
> > > arch/arm/mach-tegra/board-paz00.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/mach-tegra/board-paz00.c
> > > b/arch/arm/mach-tegra/board-paz00.c index 7478f6fb3664..ea6bff404161 100644
> > > --- a/arch/arm/mach-tegra/board-paz00.c
> > > +++ b/arch/arm/mach-tegra/board-paz00.c
> > > @@ -23,7 +23,7 @@
> > >
> > > #include "board.h"
> > >
> > > -static struct property_entry __initdata wifi_rfkill_prop[] = {
> > > +static struct property_entry wifi_rfkill_prop[] __initdata = {
> > > PROPERTY_ENTRY_STRING("name", "wifi_rfkill"),
> > > PROPERTY_ENTRY_STRING("type", "wlan"),
> > > { },
> >
> > you are right according to the documentation, but objdump -s always shows that
> > it gets put into the .rodata section. So this patch has no effect, because
> > result is always same (and wrong). It's also possible that I'm doing something
> > wrong :-) Btw, there are hundreds of such __initdata misplacement in the
> > kernel.
>
> Hmm... I get this:
>
> $ objdump -t build/tegra20/arch/arm/mach-tegra/board-paz00.o
>
> build/tegra20/arch/arm/mach-tegra/board-paz00.o: file format
> elf32-little
>
> SYMBOL TABLE:
> 00000000 l df *ABS* 00000000 board-paz00.c
> 00000000 l d .text 00000000 .text
> 00000000 l d .data 00000000 .data
> 00000000 l d .bss 00000000 .bss
> 00000000 l d .init.text 00000000 .init.text
> 00000000 l .init.text 00000000 $a
> 00000000 l .data 00000000 .LANCHOR1
> 00000000 l .init.data 00000000 .LANCHOR0
> 00000000 l d .ARM.extab.init.text 00000000 .ARM.extab.init.text
> 00000000 l d .ARM.exidx.init.text 00000000 .ARM.exidx.init.text
> 00000000 l .ARM.exidx.init.text 00000000 $d
> 00000000 l .data 00000000 $d
> 00000000 l O .data 000001c8 wifi_rfkill_device
> 000001c8 l O .data 00000048 wifi_gpio_lookup
> 00000000 l d .rodata.str1.4 00000000 .rodata.str1.4
> 00000000 l .rodata.str1.4 00000000 $d
> 00000000 l d .init.data 00000000 .init.data
> 00000000 l .init.data 00000000 $d
> 00000000 l O .init.data 00000048 wifi_rfkill_prop
> 00000000 l d .debug_frame 00000000 .debug_frame
> 00000010 l .debug_frame 00000000 $d
> 00000000 l d .debug_info 00000000 .debug_info
> 00000000 l d .debug_abbrev 00000000 .debug_abbrev
> 00000000 l d .debug_aranges 00000000 .debug_aranges
> 00000000 l d .debug_ranges 00000000 .debug_ranges
> 00000000 l d .debug_line 00000000 .debug_line
> 00000000 l d .debug_str 00000000 .debug_str
> 00000000 l d .note.GNU-stack 00000000 .note.GNU-stack
> 00000000 l d .comment 00000000 .comment
> 00000000 l d .ARM.attributes 00000000 .ARM.attributes
> 00000000 g F .init.text 00000030 tegra_paz00_wifikill_init
> 00000000 *UND* 00000000 platform_device_add_properties
> 00000000 *UND* 00000000 gpiod_add_lookup_table
> 00000000 *UND* 00000000 platform_device_register
> 00000000 *UND* 00000000 __aeabi_unwind_cpp_pr0
>
> So it correctly ends up in .init.data for me. And it does so with or
> without the patch. GCCs documentation doesn't seem to say where exactly
> these attributes need to be put, though the examples given all have the
> attribute right before the =.
>
> Oh... interestingly checkpatch does seem to have a check for this now,
> and it does recommend that __initdata be placed after wifi_rfkill_prop,
> so I'm going to apply this.
Ah, so I mis-remembered the rules and it seems that it does not really
matter if __initdata goes before or after variable name, it can't go
between "struct" and its name. From that checkpatch change introducing
the check:
static struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
does NOT put exynos4_plls in the .initdata section. The __initdata marker
can be virtually anywhere on the line, EXCEPT right after "struct". The
preferred location is before the "=" sign if there is one, or before the
trailing ";" otherwise.
so if you are applying it you might want to change the wording as to the
effect of "making checkpatch happy" which wasn't my intention, but was
the outcome ;)
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: tegra: paz00: fix __initdata placement
2017-01-23 7:43 [PATCH] ARM: tegra: paz00: fix __initdata placement Dmitry Torokhov
2017-01-23 10:04 ` Marc Dietrich
@ 2017-01-25 8:11 ` Thierry Reding
1 sibling, 0 replies; 6+ messages in thread
From: Thierry Reding @ 2017-01-25 8:11 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Stephen Warren, Alexandre Courbot, Heikki Krogerus,
linux-arm-kernel, linux-tegra, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 407 bytes --]
On Sun, Jan 22, 2017 at 11:43:47PM -0800, Dmitry Torokhov wrote:
> To have expected effect the __initdata attribute should go after variable
> name and before initializer.`
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> arch/arm/mach-tegra/board-paz00.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied with an updated commit message.
Thanks,
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-01-25 8:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-23 7:43 [PATCH] ARM: tegra: paz00: fix __initdata placement Dmitry Torokhov
2017-01-23 10:04 ` Marc Dietrich
2017-01-23 16:24 ` Thierry Reding
2017-01-24 9:08 ` Marc Dietrich
2017-01-24 18:27 ` Dmitry Torokhov
2017-01-25 8:11 ` Thierry Reding
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).