From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: Joe Perches <joe@perches.com>
Cc: Sachin Kamat <sachin.kamat@linaro.org>,
linux-arm-kernel@lists.infradead.org,
linux-samsung-soc@vger.kernel.org, kgene.kim@samsung.com,
mturquette@linaro.org, patches@linaro.org
Subject: Re: [PATCH 1/3] clk: exynos4: Make exynos4_plls static
Date: Wed, 7 Aug 2013 09:20:35 +0100 [thread overview]
Message-ID: <20130807082035.GF23006@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <1375846648.2424.69.camel@joe-AO722>
On Tue, Aug 06, 2013 at 08:37:28PM -0700, Joe Perches wrote:
> On Wed, 2013-08-07 at 08:51 +0530, Sachin Kamat wrote:
> > +CC Joe Perches
> >
> > On 7 August 2013 01:32, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> > > Also note:
> > >
> > > On Tue, Aug 06, 2013 at 05:01:13PM +0530, Sachin Kamat wrote:
> > >> @@ -984,7 +984,7 @@ static __initdata struct of_device_id ext_clk_match[] = {
> > >
> > > For the declaration above...
> > >
> > >> -struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
> > >> +static struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
> > >
> > > And this one... __initdata should come just before the '=', not at the
> > > start, not in the middle and not before the variable.
> > >
> > > The reasoning is that with how you have it above, the attributes are
> > > applied to the structure. You want to apply the attributes to the
> > > declaration instead, so it should come after the variable name.
> > >
> > > So, for example:
> > >
> > > struct foo *foo __attribute__((section(".foo"))) = (void *)1;
> > >
> > > will place the "foo" variable into a section called ".foo", but:
> > >
> > > struct __attribute__((section(".foo"))) foo *foo = (void *)1;
> > >
> > > will place "foo" into the normal .data section.
> > >
> > > So, the rule with variable declarations is that the __ specifiers we
> > > have as macros in the kernel always come after the variable name being
> > > declared and nowhere else. We consider anywhere else buggy.
> >
> > Thanks for this useful tip, Russell. There are several instances in
> > the kernel where these attributes are used at the beginning of the
> > variable declaration.
> >
> > Probably it would be useful to add this to checkpatch. Joe?
>
> I think Russell is using the royal "We".
No I'm not. There have been many patches to fix errors like the above
in the past. If it's not considered a bug by the community as a whole,
it damn well should be.
WARNING: multiple messages have this Message-ID (diff)
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/3] clk: exynos4: Make exynos4_plls static
Date: Wed, 7 Aug 2013 09:20:35 +0100 [thread overview]
Message-ID: <20130807082035.GF23006@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <1375846648.2424.69.camel@joe-AO722>
On Tue, Aug 06, 2013 at 08:37:28PM -0700, Joe Perches wrote:
> On Wed, 2013-08-07 at 08:51 +0530, Sachin Kamat wrote:
> > +CC Joe Perches
> >
> > On 7 August 2013 01:32, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> > > Also note:
> > >
> > > On Tue, Aug 06, 2013 at 05:01:13PM +0530, Sachin Kamat wrote:
> > >> @@ -984,7 +984,7 @@ static __initdata struct of_device_id ext_clk_match[] = {
> > >
> > > For the declaration above...
> > >
> > >> -struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
> > >> +static struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
> > >
> > > And this one... __initdata should come just before the '=', not at the
> > > start, not in the middle and not before the variable.
> > >
> > > The reasoning is that with how you have it above, the attributes are
> > > applied to the structure. You want to apply the attributes to the
> > > declaration instead, so it should come after the variable name.
> > >
> > > So, for example:
> > >
> > > struct foo *foo __attribute__((section(".foo"))) = (void *)1;
> > >
> > > will place the "foo" variable into a section called ".foo", but:
> > >
> > > struct __attribute__((section(".foo"))) foo *foo = (void *)1;
> > >
> > > will place "foo" into the normal .data section.
> > >
> > > So, the rule with variable declarations is that the __ specifiers we
> > > have as macros in the kernel always come after the variable name being
> > > declared and nowhere else. We consider anywhere else buggy.
> >
> > Thanks for this useful tip, Russell. There are several instances in
> > the kernel where these attributes are used at the beginning of the
> > variable declaration.
> >
> > Probably it would be useful to add this to checkpatch. Joe?
>
> I think Russell is using the royal "We".
No I'm not. There have been many patches to fix errors like the above
in the past. If it's not considered a bug by the community as a whole,
it damn well should be.
next prev parent reply other threads:[~2013-08-07 8:21 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-06 11:31 [PATCH 1/3] clk: exynos4: Make exynos4_plls static Sachin Kamat
2013-08-06 11:31 ` Sachin Kamat
2013-08-06 11:31 ` [PATCH 2/3] clk: exynos5250: Make exynos5250_plls static Sachin Kamat
2013-08-06 11:31 ` Sachin Kamat
2013-08-06 11:31 ` [PATCH 3/3] clk: exynos5420: Make exynos5420_plls static Sachin Kamat
2013-08-06 11:31 ` Sachin Kamat
2013-08-06 19:43 ` [PATCH 1/3] clk: exynos4: Make exynos4_plls static Mike Turquette
2013-08-06 19:43 ` Mike Turquette
2013-08-06 20:02 ` Russell King - ARM Linux
2013-08-06 20:02 ` Russell King - ARM Linux
2013-08-07 3:21 ` Sachin Kamat
2013-08-07 3:21 ` Sachin Kamat
2013-08-07 3:37 ` Joe Perches
2013-08-07 3:37 ` Joe Perches
2013-08-07 8:20 ` Russell King - ARM Linux [this message]
2013-08-07 8:20 ` Russell King - ARM Linux
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=20130807082035.GF23006@n2100.arm.linux.org.uk \
--to=linux@arm.linux.org.uk \
--cc=joe@perches.com \
--cc=kgene.kim@samsung.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=mturquette@linaro.org \
--cc=patches@linaro.org \
--cc=sachin.kamat@linaro.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.