Message ID | 1411777908-16016-1-git-send-email-behanw@converseincode.com |
---|---|
State | Accepted |
Commit | e8627a9ec397dd55f650e54e4956e25cfa8aab7c |
Headers | show |
On Fri, Sep 26, 2014 at 05:31:48PM -0700, Behan Webster wrote: > As written, the __init for ti_clk_get_div_table is in the middle of the return > type. > > The gcc documentation indicates that section attributes should be added to the > end of the function declaration: > > extern void foobar (void) __attribute__ ((section ("bar"))); > > However gcc seems to be very permissive with where attributes can be placed. > clang on the other hand isn't so permissive, and fails if you put the section > definition in the middle of the return type: > > drivers/clk/ti/divider.c:298:28: error: expected ';' after struct > static struct clk_div_table > ^ > ; > drivers/clk/ti/divider.c:298:1: warning: 'static' ignored on this > declaration [-Wmissing-declarations] > static struct clk_div_table > ^ > drivers/clk/ti/divider.c:299:9: error: type specifier missing, > defaults to 'int' [-Werror,-Wimplicit-int] > __init *ti_clk_get_div_table(struct device_node *node) > ~~~~~~ ^ > drivers/clk/ti/divider.c:345:9: warning: incompatible pointer types > returning 'struct clk_div_table *' from a function with result type 'int *' [-Wincompatible-pointer-types] > return table; > ^~~~~ > drivers/clk/ti/divider.c:419:9: warning: incompatible pointer types > assigning to 'const struct clk_div_table *' from 'int *' [-Wincompatible-pointer-types] > *table = ti_clk_get_div_table(node); > ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~ > 3 warnings and 2 errors generated. > > By convention, most of the kernel code puts section attributes between the > return type and function name. In the case where the return type is a pointer, > it's important to place the '*' on left of the __init. > > This updated code works for both gcc and clang. > > Signed-off-by: Behan Webster <behanw@converseincode.com> > Reviewed-by: Mark Charlebois <charlebm@gmail.com> makes sense to me: Reviewed-by: Felipe Balbi <balbi@ti.com> I wonder if we should add this a Sparse or Coccinelle rule.
On 09/26/14 17:55, Felipe Balbi wrote: > On Fri, Sep 26, 2014 at 05:31:48PM -0700, Behan Webster wrote: >> As written, the __init for ti_clk_get_div_table is in the middle of the return >> type. >> >> The gcc documentation indicates that section attributes should be added to the >> end of the function declaration: >> >> extern void foobar (void) __attribute__ ((section ("bar"))); >> >> However gcc seems to be very permissive with where attributes can be placed. >> clang on the other hand isn't so permissive, and fails if you put the section >> definition in the middle of the return type: >> >> drivers/clk/ti/divider.c:298:28: error: expected ';' after struct >> static struct clk_div_table >> ^ >> ; >> drivers/clk/ti/divider.c:298:1: warning: 'static' ignored on this >> declaration [-Wmissing-declarations] >> static struct clk_div_table >> ^ >> drivers/clk/ti/divider.c:299:9: error: type specifier missing, >> defaults to 'int' [-Werror,-Wimplicit-int] >> __init *ti_clk_get_div_table(struct device_node *node) >> ~~~~~~ ^ >> drivers/clk/ti/divider.c:345:9: warning: incompatible pointer types >> returning 'struct clk_div_table *' from a function with result type 'int *' [-Wincompatible-pointer-types] >> return table; >> ^~~~~ >> drivers/clk/ti/divider.c:419:9: warning: incompatible pointer types >> assigning to 'const struct clk_div_table *' from 'int *' [-Wincompatible-pointer-types] >> *table = ti_clk_get_div_table(node); >> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~ >> 3 warnings and 2 errors generated. >> >> By convention, most of the kernel code puts section attributes between the >> return type and function name. In the case where the return type is a pointer, >> it's important to place the '*' on left of the __init. >> >> This updated code works for both gcc and clang. >> >> Signed-off-by: Behan Webster <behanw@converseincode.com> >> Reviewed-by: Mark Charlebois <charlebm@gmail.com> > makes sense to me: > > Reviewed-by: Felipe Balbi <balbi@ti.com> Thank you. > I wonder if we should add this a Sparse or Coccinelle rule. +1 I'm hoping it can be added to checkpatch as well. Behan
On 09/27/2014 03:57 AM, Behan Webster wrote: > On 09/26/14 17:55, Felipe Balbi wrote: >> On Fri, Sep 26, 2014 at 05:31:48PM -0700, Behan Webster wrote: >>> As written, the __init for ti_clk_get_div_table is in the middle of >>> the return >>> type. >>> >>> The gcc documentation indicates that section attributes should be >>> added to the >>> end of the function declaration: >>> >>> extern void foobar (void) __attribute__ ((section ("bar"))); >>> >>> However gcc seems to be very permissive with where attributes can be >>> placed. >>> clang on the other hand isn't so permissive, and fails if you put the >>> section >>> definition in the middle of the return type: >>> >>> drivers/clk/ti/divider.c:298:28: error: expected ';' after struct >>> static struct clk_div_table >>> ^ >>> ; >>> drivers/clk/ti/divider.c:298:1: warning: 'static' ignored on this >>> declaration [-Wmissing-declarations] >>> static struct clk_div_table >>> ^ >>> drivers/clk/ti/divider.c:299:9: error: type specifier missing, >>> defaults to 'int' [-Werror,-Wimplicit-int] >>> __init *ti_clk_get_div_table(struct device_node *node) >>> ~~~~~~ ^ >>> drivers/clk/ti/divider.c:345:9: warning: incompatible pointer types >>> returning 'struct clk_div_table *' from a function with result >>> type 'int *' [-Wincompatible-pointer-types] >>> return table; >>> ^~~~~ >>> drivers/clk/ti/divider.c:419:9: warning: incompatible pointer types >>> assigning to 'const struct clk_div_table *' from 'int *' >>> [-Wincompatible-pointer-types] >>> *table = ti_clk_get_div_table(node); >>> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> 3 warnings and 2 errors generated. >>> >>> By convention, most of the kernel code puts section attributes >>> between the >>> return type and function name. In the case where the return type is a >>> pointer, >>> it's important to place the '*' on left of the __init. >>> >>> This updated code works for both gcc and clang. >>> >>> Signed-off-by: Behan Webster <behanw@converseincode.com> >>> Reviewed-by: Mark Charlebois <charlebm@gmail.com> >> makes sense to me: >> >> Reviewed-by: Felipe Balbi <balbi@ti.com> > Thank you. > >> I wonder if we should add this a Sparse or Coccinelle rule. > +1 > > I'm hoping it can be added to checkpatch as well. > > Behan > Thanks, patch applied to for-v3.18/ti-clk-drv. -Tero -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
diff --git a/drivers/clk/ti/divider.c b/drivers/clk/ti/divider.c index a837f70..bff2b5b 100644 --- a/drivers/clk/ti/divider.c +++ b/drivers/clk/ti/divider.c @@ -300,8 +300,8 @@ static struct clk *_register_divider(struct device *dev, const char *name, return clk; } -static struct clk_div_table -__init *ti_clk_get_div_table(struct device_node *node) +static struct clk_div_table * +__init ti_clk_get_div_table(struct device_node *node) { struct clk_div_table *table; const __be32 *divspec;