All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
To: David Laight <David.Laight@aculab.com>
Cc: Marcelo Tosatti <marcelo@kvack.org>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] powerpc/8xx: tqm8xx: fix incorrect placement of __initdata tag
Date: Mon, 30 Sep 2013 17:05:51 +0200	[thread overview]
Message-ID: <9412116.FMJVEO7gkK@amdc1032> (raw)
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B736B@saturn3.aculab.com>

On Monday, September 30, 2013 03:20:29 PM David Laight 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.
> ...
> > -static struct __initdata cpm_pin tqm8xx_pins[] = {
> > +static struct cpm_pin tqm8xx_pins[] __initdata = {
> 
> As far as gcc is concerned it can go almost anywhere before the '=',
> even before the 'static'.
> Splitting 'struct cpm_pin' does seem an odd choice.

It is not only an odd choice, it just doesn't work as it should in
the practice (as tested with gcc-4.6.3 from Ubuntu 12.04).

> The Linux coding standards might suggest a location.
> I'd have thought that either before or after the 'static' would be best
> (ie as a storage class qualifier).

The majority of the kernel code uses __initdata before equal sign
and the __initdata documentation in <linux/init.h> recommends such
usage:

"
 * For initialized data:
 * You should insert __initdata or __initconst between the variable name
 * and equal sign followed by value, e.g.:
 *
 * static int init_variable __initdata = 0;
 * static const char linux_logo[] __initconst = { 0x32, 0x36, ... };
"

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

WARNING: multiple messages have this Message-ID (diff)
From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
To: David Laight <David.Laight@aculab.com>
Cc: Vitaly Bordug <vitb@kernel.crashing.org>,
	Marcelo Tosatti <marcelo@kvack.org>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] powerpc/8xx: tqm8xx: fix incorrect placement of __initdata tag
Date: Mon, 30 Sep 2013 17:05:51 +0200	[thread overview]
Message-ID: <9412116.FMJVEO7gkK@amdc1032> (raw)
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B736B@saturn3.aculab.com>

On Monday, September 30, 2013 03:20:29 PM David Laight 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.
> ...
> > -static struct __initdata cpm_pin tqm8xx_pins[] = {
> > +static struct cpm_pin tqm8xx_pins[] __initdata = {
> 
> As far as gcc is concerned it can go almost anywhere before the '=',
> even before the 'static'.
> Splitting 'struct cpm_pin' does seem an odd choice.

It is not only an odd choice, it just doesn't work as it should in
the practice (as tested with gcc-4.6.3 from Ubuntu 12.04).

> The Linux coding standards might suggest a location.
> I'd have thought that either before or after the 'static' would be best
> (ie as a storage class qualifier).

The majority of the kernel code uses __initdata before equal sign
and the __initdata documentation in <linux/init.h> recommends such
usage:

"
 * For initialized data:
 * You should insert __initdata or __initconst between the variable name
 * and equal sign followed by value, e.g.:
 *
 * static int init_variable __initdata = 0;
 * static const char linux_logo[] __initconst = { 0x32, 0x36, ... };
"

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


  reply	other threads:[~2013-09-30 15:06 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-30 13:13 [PATCH] powerpc/8xx: tqm8xx: fix incorrect placement of __initdata tag Bartlomiej Zolnierkiewicz
2013-09-30 13:13 ` Bartlomiej Zolnierkiewicz
2013-09-30 14:20 ` David Laight
2013-09-30 14:20   ` David Laight
2013-09-30 15:05   ` Bartlomiej Zolnierkiewicz [this message]
2013-09-30 15:05     ` Bartlomiej Zolnierkiewicz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9412116.FMJVEO7gkK@amdc1032 \
    --to=b.zolnierkie@samsung.com \
    --cc=David.Laight@aculab.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=marcelo@kvack.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.