diff mbox

clk, ti, LLVMLinux: Move __init outside of type definition

Message ID 1411777908-16016-1-git-send-email-behanw@converseincode.com
State Accepted
Commit e8627a9ec397dd55f650e54e4956e25cfa8aab7c
Headers show

Commit Message

Behan Webster Sept. 27, 2014, 12:31 a.m. UTC
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>
---
 drivers/clk/ti/divider.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Felipe Balbi Sept. 27, 2014, 12:55 a.m. UTC | #1
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.
Behan Webster Sept. 27, 2014, 12:57 a.m. UTC | #2
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
Tero Kristo Sept. 29, 2014, 11:44 a.m. UTC | #3
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 mbox

Patch

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;