[PATCH] ARM: ks8695: fix __initdata annotation

[PATCH] ARM: ks8695: fix __initdata annotation

[Arnd Bergmann]

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

Re: [PATCH] ARM: ks8695: fix __initdata annotation

[Greg Ungerer]

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),
> 

Re: [PATCH] ARM: ks8695: fix __initdata annotation

[Uwe Kleine-König]

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/  |

Re: [PATCH] ARM: ks8695: fix __initdata annotation

[Arnd Bergmann]

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

Re: [PATCH] ARM: ks8695: fix __initdata annotation

[Uwe Kleine-König]

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/  |

Re: [PATCH] ARM: ks8695: fix __initdata annotation

[Arnd Bergmann]

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

Re: [PATCH] ARM: ks8695: fix __initdata annotation

[Arnd Bergmann]

On Tuesday 09 February 2016 09:36:00 Greg Ungerer wrote:
> 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>
> 

Applied to next/fixes-non-critical in arm-soc directly.

Thanks,

	Arnd