Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1329103 > unrolled thread

[PATCH] ARM: ks8695: fix __initdata annotation

Started byArnd Bergmann <arnd@arndb.de>
First post2016-02-08 15:30 +0100
Last post2016-02-09 16:20 +0100
Articles 6 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] ARM: ks8695: fix __initdata annotation Arnd Bergmann <arnd@arndb.de> - 2016-02-08 15:30 +0100
    Re: [PATCH] ARM: ks8695: fix __initdata annotation Greg Ungerer <gerg@uclinux.org> - 2016-02-09 00:40 +0100
    Re: [PATCH] ARM: ks8695: fix __initdata annotation Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-02-09 10:10 +0100
      Re: [PATCH] ARM: ks8695: fix __initdata annotation Arnd Bergmann <arnd@arndb.de> - 2016-02-09 12:20 +0100
        Re: [PATCH] ARM: ks8695: fix __initdata annotation Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-02-09 12:40 +0100
          Re: [PATCH] ARM: ks8695: fix __initdata annotation Arnd Bergmann <arnd@arndb.de> - 2016-02-09 16:20 +0100

#1329103 — [PATCH] ARM: ks8695: fix __initdata annotation

FromArnd Bergmann <arnd@arndb.de>
Date2016-02-08 15:30 +0100
Subject[PATCH] ARM: ks8695: fix __initdata annotation
Message-ID<qZUZc-6vM-3@gated-at.bofh.it>
Clang complains about the __initdata section attribute being in the
wrong place in two files of ks8695:

arch/arm/mach-ks8695/cpu.c:37:31: error: '__section__' attribute only applies to functions and global variables
arch/arm/mach-ks8695/board-og.c:83:31: error: '__section__' attribute only applies to functions and global variables

This moves the attribute to the correct place.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mach-ks8695/board-og.c | 2 +-
 arch/arm/mach-ks8695/cpu.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-ks8695/board-og.c b/arch/arm/mach-ks8695/board-og.c
index 1f4f2f4f25bb..fa1a7c2ca2bb 100644
--- a/arch/arm/mach-ks8695/board-og.c
+++ b/arch/arm/mach-ks8695/board-og.c
@@ -80,7 +80,7 @@ static void __init og_pci_bus_reset(void)
 #define	S8250_VIRT	0xf4000000
 #define	S8250_SIZE	0x00100000
 
-static struct __initdata map_desc og_io_desc[] = {
+static struct map_desc __initdata og_io_desc[] = {
 	{
 		.virtual	= S8250_VIRT,
 		.pfn		= __phys_to_pfn(S8250_PHYS),
diff --git a/arch/arm/mach-ks8695/cpu.c b/arch/arm/mach-ks8695/cpu.c
index 474a050da85b..f56937890e3a 100644
--- a/arch/arm/mach-ks8695/cpu.c
+++ b/arch/arm/mach-ks8695/cpu.c
@@ -34,7 +34,7 @@
 #include <mach/regs-misc.h>
 
 
-static struct __initdata map_desc ks8695_io_desc[] = {
+static struct map_desc __initdata ks8695_io_desc[] = {
 	{
 		.virtual	= (unsigned long)KS8695_IO_VA,
 		.pfn		= __phys_to_pfn(KS8695_IO_PA),
-- 
2.7.0

[toc] | [next] | [standalone]


#1329689

FromGreg Ungerer <gerg@uclinux.org>
Date2016-02-09 00:40 +0100
Message-ID<r03zs-3Mr-11@gated-at.bofh.it>
In reply to#1329103
On 09/02/16 00:24, Arnd Bergmann wrote:
> Clang complains about the __initdata section attribute being in the
> wrong place in two files of ks8695:
> 
> arch/arm/mach-ks8695/cpu.c:37:31: error: '__section__' attribute only applies to functions and global variables
> arch/arm/mach-ks8695/board-og.c:83:31: error: '__section__' attribute only applies to functions and global variables
> 
> This moves the attribute to the correct place.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Greg Ungerer <gerg@uclinux.org>

Regards
Greg


> ---
>  arch/arm/mach-ks8695/board-og.c | 2 +-
>  arch/arm/mach-ks8695/cpu.c      | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-ks8695/board-og.c b/arch/arm/mach-ks8695/board-og.c
> index 1f4f2f4f25bb..fa1a7c2ca2bb 100644
> --- a/arch/arm/mach-ks8695/board-og.c
> +++ b/arch/arm/mach-ks8695/board-og.c
> @@ -80,7 +80,7 @@ static void __init og_pci_bus_reset(void)
>  #define	S8250_VIRT	0xf4000000
>  #define	S8250_SIZE	0x00100000
>  
> -static struct __initdata map_desc og_io_desc[] = {
> +static struct map_desc __initdata og_io_desc[] = {
>  	{
>  		.virtual	= S8250_VIRT,
>  		.pfn		= __phys_to_pfn(S8250_PHYS),
> diff --git a/arch/arm/mach-ks8695/cpu.c b/arch/arm/mach-ks8695/cpu.c
> index 474a050da85b..f56937890e3a 100644
> --- a/arch/arm/mach-ks8695/cpu.c
> +++ b/arch/arm/mach-ks8695/cpu.c
> @@ -34,7 +34,7 @@
>  #include <mach/regs-misc.h>
>  
>  
> -static struct __initdata map_desc ks8695_io_desc[] = {
> +static struct map_desc __initdata ks8695_io_desc[] = {
>  	{
>  		.virtual	= (unsigned long)KS8695_IO_VA,
>  		.pfn		= __phys_to_pfn(KS8695_IO_PA),
> 

[toc] | [prev] | [next] | [standalone]


#1329974

FromUwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date2016-02-09 10:10 +0100
Message-ID<r0ct4-1XM-29@gated-at.bofh.it>
In reply to#1329103
On Mon, Feb 08, 2016 at 03:24:57PM +0100, Arnd Bergmann wrote:
> Clang complains about the __initdata section attribute being in the
> wrong place in two files of ks8695:
> 
> arch/arm/mach-ks8695/cpu.c:37:31: error: '__section__' attribute only applies to functions and global variables
> arch/arm/mach-ks8695/board-og.c:83:31: error: '__section__' attribute only applies to functions and global variables
> 
> This moves the attribute to the correct place.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/mach-ks8695/board-og.c | 2 +-
>  arch/arm/mach-ks8695/cpu.c      | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-ks8695/board-og.c b/arch/arm/mach-ks8695/board-og.c
> index 1f4f2f4f25bb..fa1a7c2ca2bb 100644
> --- a/arch/arm/mach-ks8695/board-og.c
> +++ b/arch/arm/mach-ks8695/board-og.c
> @@ -80,7 +80,7 @@ static void __init og_pci_bus_reset(void)
>  #define	S8250_VIRT	0xf4000000
>  #define	S8250_SIZE	0x00100000
>  
> -static struct __initdata map_desc og_io_desc[] = {
> +static struct map_desc __initdata og_io_desc[] = {

I would have expected that

+static struct map_desc og_io_desc[] __initdata = {

is the correct variant?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[toc] | [prev] | [next] | [standalone]


#1330104

FromArnd Bergmann <arnd@arndb.de>
Date2016-02-09 12:20 +0100
Message-ID<r0euS-3fE-43@gated-at.bofh.it>
In reply to#1329974
On Tuesday 09 February 2016 10:00:30 Uwe Kleine-König wrote:
> > diff --git a/arch/arm/mach-ks8695/board-og.c b/arch/arm/mach-ks8695/board-og.c
> > index 1f4f2f4f25bb..fa1a7c2ca2bb 100644
> > --- a/arch/arm/mach-ks8695/board-og.c
> > +++ b/arch/arm/mach-ks8695/board-og.c
> > @@ -80,7 +80,7 @@ static void __init og_pci_bus_reset(void)
> >  #define      S8250_VIRT      0xf4000000
> >  #define      S8250_SIZE      0x00100000
> >  
> > -static struct __initdata map_desc og_io_desc[] = {
> > +static struct map_desc __initdata og_io_desc[] = {
> 
> I would have expected that
> 
> +static struct map_desc og_io_desc[] __initdata = {
> 
> is the correct variant?
> 

I think those two mean the exact same thing, and we have tons of examples
for either one in the kernel, unlike the one I removed. I have
verified that the resulting object files are identical.

Can you point me to some documentation that clarifies which one to use,
and why?

	Arnd

[toc] | [prev] | [next] | [standalone]


#1330117

FromUwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date2016-02-09 12:40 +0100
Message-ID<r0eOe-3mG-13@gated-at.bofh.it>
In reply to#1330104
Hello Arnd,

On Tue, Feb 09, 2016 at 12:14:15PM +0100, Arnd Bergmann wrote:
> On Tuesday 09 February 2016 10:00:30 Uwe Kleine-König wrote:
> > > diff --git a/arch/arm/mach-ks8695/board-og.c b/arch/arm/mach-ks8695/board-og.c
> > > index 1f4f2f4f25bb..fa1a7c2ca2bb 100644
> > > --- a/arch/arm/mach-ks8695/board-og.c
> > > +++ b/arch/arm/mach-ks8695/board-og.c
> > > @@ -80,7 +80,7 @@ static void __init og_pci_bus_reset(void)
> > >  #define      S8250_VIRT      0xf4000000
> > >  #define      S8250_SIZE      0x00100000
> > >  
> > > -static struct __initdata map_desc og_io_desc[] = {
> > > +static struct map_desc __initdata og_io_desc[] = {
> > 
> > I would have expected that
> > 
> > +static struct map_desc og_io_desc[] __initdata = {
> > 
> > is the correct variant?
> > 
> 
> I think those two mean the exact same thing, and we have tons of examples
> for either one in the kernel, unlike the one I removed. I have
> verified that the resulting object files are identical.
> 
> Can you point me to some documentation that clarifies which one to use,
> and why?

Having the attribute list after the declarator isn't recommended as
explicit as I remember having read it somewhere in the gcc docs.

	info gcc "Attribute Syntax"

has:

	 An attribute specifier list may appear immediately before a declarator
	(other than the first) in a comma-separated list of declarators in a
	declaration of more than one identifier using a single list of
	specifiers and qualifiers.  Such attribute specifiers apply only to the
	identifier before whose declarator they appear.  For example, in

	     __attribute__((noreturn)) void d0 (void),
		 __attribute__((format(printf, 1, 2))) d1 (const char *, ...),
		  d2 (void)

	the 'noreturn' attribute applies to all the functions declared; the
	'format' attribute only applies to 'd1'.

(Funny enough, in the example the attribute specifier list doesn't
appear *immediately* before the declarator d0.)

This might be interpreted as "usually the attribute specifier list appears
after the declarator". Other than that I cannot find an explict
recommended placement in the docs. The examples in

	info gcc "Variable Attributes"

always have the attribute list after the declarator.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[toc] | [prev] | [next] | [standalone]


#1330340

FromArnd Bergmann <arnd@arndb.de>
Date2016-02-09 16:20 +0100
Message-ID<r0if9-5Nd-49@gated-at.bofh.it>
In reply to#1330117
On Tuesday 09 February 2016 12:36:24 Uwe Kleine-König wrote:
> Having the attribute list after the declarator isn't recommended as
> explicit as I remember having read it somewhere in the gcc docs.
> 
>         info gcc "Attribute Syntax"
> 
> has:
> 
>          An attribute specifier list may appear immediately before a declarator
>         (other than the first) in a comma-separated list of declarators in a
>         declaration of more than one identifier using a single list of
>         specifiers and qualifiers.  Such attribute specifiers apply only to the
>         identifier before whose declarator they appear.  For example, in
> 
>              __attribute__((noreturn)) void d0 (void),
>                  __attribute__((format(printf, 1, 2))) d1 (const char *, ...),
>                   d2 (void)
> 
>         the 'noreturn' attribute applies to all the functions declared; the
>         'format' attribute only applies to 'd1'.
> 
> (Funny enough, in the example the attribute specifier list doesn't
> appear *immediately* before the declarator d0.)
> 
> This might be interpreted as "usually the attribute specifier list appears
> after the declarator". Other than that I cannot find an explict
> recommended placement in the docs. The examples in
> 
>         info gcc "Variable Attributes"
> 
> always have the attribute list after the declarator.

Ok, thanks for the detailed answer, I've fixed it up locally now.
I guess I'm applying the patch in arm-soc directly at some point
and will use the line you suggested after some more testing:

 static struct map_desc og_io_desc[] __initdata = {


	Arnd

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web