* [PATCH] Don't mark shared helper functions as inline
@ 2012-11-26 19:20 Russ Dill
2012-11-26 19:57 ` Mike Turquette
0 siblings, 1 reply; 5+ messages in thread
From: Russ Dill @ 2012-11-26 19:20 UTC (permalink / raw)
To: linux-omap, linux-arm-kernel, linux-kernel; +Cc: mturquette, Russ Dill
The helper functions that access the opaque struct clk should
not be marked inline since they are contained in clk.c, but expected
to be used by other compilation units. This causes compile errors
under gcc-4.7
In file included from arch/arm/mach-omap2/clockdomain.c:25:0:
arch/arm/mach-omap2/clockdomain.c: In function ‘clkdm_clk_disable’:
include/linux/clk-provider.h:338:12: error: inlining failed in call to always_inline ‘__clk_get_enable_count’: function body not available
arch/arm/mach-omap2/clockdomain.c:1001:28: error: called from here
make[1]: *** [arch/arm/mach-omap2/clockdomain.o] Error 1
make: *** [arch/arm/mach-omap2] Error 2
Signed-off-by: Russ Dill <Russ.Dill@ti.com>
---
drivers/clk/clk.c | 14 +++++++-------
include/linux/clk-provider.h | 4 ++--
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 56e4495e..ed01746 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -249,32 +249,32 @@ late_initcall(clk_disable_unused);
/*** helper functions ***/
-inline const char *__clk_get_name(struct clk *clk)
+const char *__clk_get_name(struct clk *clk)
{
return !clk ? NULL : clk->name;
}
-inline struct clk_hw *__clk_get_hw(struct clk *clk)
+struct clk_hw *__clk_get_hw(struct clk *clk)
{
return !clk ? NULL : clk->hw;
}
-inline u8 __clk_get_num_parents(struct clk *clk)
+u8 __clk_get_num_parents(struct clk *clk)
{
return !clk ? -EINVAL : clk->num_parents;
}
-inline struct clk *__clk_get_parent(struct clk *clk)
+struct clk *__clk_get_parent(struct clk *clk)
{
return !clk ? NULL : clk->parent;
}
-inline int __clk_get_enable_count(struct clk *clk)
+int __clk_get_enable_count(struct clk *clk)
{
return !clk ? -EINVAL : clk->enable_count;
}
-inline int __clk_get_prepare_count(struct clk *clk)
+int __clk_get_prepare_count(struct clk *clk)
{
return !clk ? -EINVAL : clk->prepare_count;
}
@@ -300,7 +300,7 @@ out:
return ret;
}
-inline unsigned long __clk_get_flags(struct clk *clk)
+unsigned long __clk_get_flags(struct clk *clk)
{
return !clk ? -EINVAL : clk->flags;
}
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index c127315..f9f5e9e 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -335,8 +335,8 @@ const char *__clk_get_name(struct clk *clk);
struct clk_hw *__clk_get_hw(struct clk *clk);
u8 __clk_get_num_parents(struct clk *clk);
struct clk *__clk_get_parent(struct clk *clk);
-inline int __clk_get_enable_count(struct clk *clk);
-inline int __clk_get_prepare_count(struct clk *clk);
+int __clk_get_enable_count(struct clk *clk);
+int __clk_get_prepare_count(struct clk *clk);
unsigned long __clk_get_rate(struct clk *clk);
unsigned long __clk_get_flags(struct clk *clk);
int __clk_is_enabled(struct clk *clk);
--
1.8.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Don't mark shared helper functions as inline
2012-11-26 19:20 [PATCH] Don't mark shared helper functions as inline Russ Dill
@ 2012-11-26 19:57 ` Mike Turquette
2012-11-26 21:00 ` Mark A. Greer
2012-11-27 17:52 ` Russ Dill
0 siblings, 2 replies; 5+ messages in thread
From: Mike Turquette @ 2012-11-26 19:57 UTC (permalink / raw)
To: Russ Dill, linux-omap, linux-arm-kernel, linux-kernel; +Cc: Russ Dill
Quoting Russ Dill (2012-11-26 11:20:09)
> The helper functions that access the opaque struct clk should
> not be marked inline since they are contained in clk.c, but expected
> to be used by other compilation units. This causes compile errors
> under gcc-4.7
>
> In file included from arch/arm/mach-omap2/clockdomain.c:25:0:
> arch/arm/mach-omap2/clockdomain.c: In function ‘clkdm_clk_disable’:
> include/linux/clk-provider.h:338:12: error: inlining failed in call to always_inline ‘__clk_get_enable_count’: function body not available
> arch/arm/mach-omap2/clockdomain.c:1001:28: error: called from here
> make[1]: *** [arch/arm/mach-omap2/clockdomain.o] Error 1
> make: *** [arch/arm/mach-omap2] Error 2
>
Hi Russ,
A fix for this was merged into rc7. See 93532c8a, "clk: remove inline
usage from clk-provider.h".
Regardless, I'm still considering this patch. I've heard many times
that we should trust the compiler to optimize for us and some folks look
down on inlining in general. If anyone has an opinion on removing
inlines from the common clk core then please do speak up.
Russ, can you update to the latest rc and verify if that fix is enough
for you?
Regards,
Mike
>
> Signed-off-by: Russ Dill <Russ.Dill@ti.com>
> ---
> drivers/clk/clk.c | 14 +++++++-------
> include/linux/clk-provider.h | 4 ++--
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 56e4495e..ed01746 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -249,32 +249,32 @@ late_initcall(clk_disable_unused);
>
> /*** helper functions ***/
>
> -inline const char *__clk_get_name(struct clk *clk)
> +const char *__clk_get_name(struct clk *clk)
> {
> return !clk ? NULL : clk->name;
> }
>
> -inline struct clk_hw *__clk_get_hw(struct clk *clk)
> +struct clk_hw *__clk_get_hw(struct clk *clk)
> {
> return !clk ? NULL : clk->hw;
> }
>
> -inline u8 __clk_get_num_parents(struct clk *clk)
> +u8 __clk_get_num_parents(struct clk *clk)
> {
> return !clk ? -EINVAL : clk->num_parents;
> }
>
> -inline struct clk *__clk_get_parent(struct clk *clk)
> +struct clk *__clk_get_parent(struct clk *clk)
> {
> return !clk ? NULL : clk->parent;
> }
>
> -inline int __clk_get_enable_count(struct clk *clk)
> +int __clk_get_enable_count(struct clk *clk)
> {
> return !clk ? -EINVAL : clk->enable_count;
> }
>
> -inline int __clk_get_prepare_count(struct clk *clk)
> +int __clk_get_prepare_count(struct clk *clk)
> {
> return !clk ? -EINVAL : clk->prepare_count;
> }
> @@ -300,7 +300,7 @@ out:
> return ret;
> }
>
> -inline unsigned long __clk_get_flags(struct clk *clk)
> +unsigned long __clk_get_flags(struct clk *clk)
> {
> return !clk ? -EINVAL : clk->flags;
> }
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index c127315..f9f5e9e 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -335,8 +335,8 @@ const char *__clk_get_name(struct clk *clk);
> struct clk_hw *__clk_get_hw(struct clk *clk);
> u8 __clk_get_num_parents(struct clk *clk);
> struct clk *__clk_get_parent(struct clk *clk);
> -inline int __clk_get_enable_count(struct clk *clk);
> -inline int __clk_get_prepare_count(struct clk *clk);
> +int __clk_get_enable_count(struct clk *clk);
> +int __clk_get_prepare_count(struct clk *clk);
> unsigned long __clk_get_rate(struct clk *clk);
> unsigned long __clk_get_flags(struct clk *clk);
> int __clk_is_enabled(struct clk *clk);
> --
> 1.8.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Don't mark shared helper functions as inline
2012-11-26 19:57 ` Mike Turquette
@ 2012-11-26 21:00 ` Mark A. Greer
2012-11-27 17:52 ` Russ Dill
1 sibling, 0 replies; 5+ messages in thread
From: Mark A. Greer @ 2012-11-26 21:00 UTC (permalink / raw)
To: Mike Turquette; +Cc: Russ Dill, linux-omap, linux-arm-kernel, linux-kernel
On Mon, Nov 26, 2012 at 11:57:42AM -0800, Mike Turquette wrote:
> Quoting Russ Dill (2012-11-26 11:20:09)
> > The helper functions that access the opaque struct clk should
> > not be marked inline since they are contained in clk.c, but expected
> > to be used by other compilation units. This causes compile errors
> > under gcc-4.7
> >
> > In file included from arch/arm/mach-omap2/clockdomain.c:25:0:
> > arch/arm/mach-omap2/clockdomain.c: In function ‘clkdm_clk_disable’:
> > include/linux/clk-provider.h:338:12: error: inlining failed in call to always_inline ‘__clk_get_enable_count’: function body not available
> > arch/arm/mach-omap2/clockdomain.c:1001:28: error: called from here
> > make[1]: *** [arch/arm/mach-omap2/clockdomain.o] Error 1
> > make: *** [arch/arm/mach-omap2] Error 2
> >
>
Hi Mike.
> Regardless, I'm still considering this patch. I've heard many times
> that we should trust the compiler to optimize for us and some folks look
> down on inlining in general. If anyone has an opinion on removing
> inlines from the common clk core then please do speak up.
You should take a look at "Chapter 15: The inline disease" in
Documentation/CodingStyle.
Mark
--
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Don't mark shared helper functions as inline
2012-11-26 19:57 ` Mike Turquette
2012-11-26 21:00 ` Mark A. Greer
@ 2012-11-27 17:52 ` Russ Dill
2013-01-12 0:37 ` Mike Turquette
1 sibling, 1 reply; 5+ messages in thread
From: Russ Dill @ 2012-11-27 17:52 UTC (permalink / raw)
To: Mike Turquette; +Cc: linux-omap, Linux ARM Kernel List, linux-kernel
On Mon, Nov 26, 2012 at 11:57 AM, Mike Turquette <mturquette@ti.com> wrote:
>
> Quoting Russ Dill (2012-11-26 11:20:09)
> > The helper functions that access the opaque struct clk should
> > not be marked inline since they are contained in clk.c, but expected
> > to be used by other compilation units. This causes compile errors
> > under gcc-4.7
> >
> > In file included from arch/arm/mach-omap2/clockdomain.c:25:0:
> > arch/arm/mach-omap2/clockdomain.c: In function ‘clkdm_clk_disable’:
> > include/linux/clk-provider.h:338:12: error: inlining failed in call to always_inline ‘__clk_get_enable_count’: function body not available
> > arch/arm/mach-omap2/clockdomain.c:1001:28: error: called from here
> > make[1]: *** [arch/arm/mach-omap2/clockdomain.o] Error 1
> > make: *** [arch/arm/mach-omap2] Error 2
> >
>
> Hi Russ,
>
> A fix for this was merged into rc7. See 93532c8a, "clk: remove inline
> usage from clk-provider.h".
>
> Regardless, I'm still considering this patch. I've heard many times
> that we should trust the compiler to optimize for us and some folks look
> down on inlining in general. If anyone has an opinion on removing
> inlines from the common clk core then please do speak up.
>
> Russ, can you update to the latest rc and verify if that fix is enough
> for you?
Yes, that commit fixes the compile issue too. I'd go with the more
complete removal of inlines though. If you have a declaration in a
header, it makes no sense to call it inline in the .c
> Regards,
> Mike
>
> >
> > Signed-off-by: Russ Dill <Russ.Dill@ti.com>
> > ---
> > drivers/clk/clk.c | 14 +++++++-------
> > include/linux/clk-provider.h | 4 ++--
> > 2 files changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index 56e4495e..ed01746 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -249,32 +249,32 @@ late_initcall(clk_disable_unused);
> >
> > /*** helper functions ***/
> >
> > -inline const char *__clk_get_name(struct clk *clk)
> > +const char *__clk_get_name(struct clk *clk)
> > {
> > return !clk ? NULL : clk->name;
> > }
> >
> > -inline struct clk_hw *__clk_get_hw(struct clk *clk)
> > +struct clk_hw *__clk_get_hw(struct clk *clk)
> > {
> > return !clk ? NULL : clk->hw;
> > }
> >
> > -inline u8 __clk_get_num_parents(struct clk *clk)
> > +u8 __clk_get_num_parents(struct clk *clk)
> > {
> > return !clk ? -EINVAL : clk->num_parents;
> > }
> >
> > -inline struct clk *__clk_get_parent(struct clk *clk)
> > +struct clk *__clk_get_parent(struct clk *clk)
> > {
> > return !clk ? NULL : clk->parent;
> > }
> >
> > -inline int __clk_get_enable_count(struct clk *clk)
> > +int __clk_get_enable_count(struct clk *clk)
> > {
> > return !clk ? -EINVAL : clk->enable_count;
> > }
> >
> > -inline int __clk_get_prepare_count(struct clk *clk)
> > +int __clk_get_prepare_count(struct clk *clk)
> > {
> > return !clk ? -EINVAL : clk->prepare_count;
> > }
> > @@ -300,7 +300,7 @@ out:
> > return ret;
> > }
> >
> > -inline unsigned long __clk_get_flags(struct clk *clk)
> > +unsigned long __clk_get_flags(struct clk *clk)
> > {
> > return !clk ? -EINVAL : clk->flags;
> > }
> > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> > index c127315..f9f5e9e 100644
> > --- a/include/linux/clk-provider.h
> > +++ b/include/linux/clk-provider.h
> > @@ -335,8 +335,8 @@ const char *__clk_get_name(struct clk *clk);
> > struct clk_hw *__clk_get_hw(struct clk *clk);
> > u8 __clk_get_num_parents(struct clk *clk);
> > struct clk *__clk_get_parent(struct clk *clk);
> > -inline int __clk_get_enable_count(struct clk *clk);
> > -inline int __clk_get_prepare_count(struct clk *clk);
> > +int __clk_get_enable_count(struct clk *clk);
> > +int __clk_get_prepare_count(struct clk *clk);
> > unsigned long __clk_get_rate(struct clk *clk);
> > unsigned long __clk_get_flags(struct clk *clk);
> > int __clk_is_enabled(struct clk *clk);
> > --
> > 1.8.0
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Don't mark shared helper functions as inline
2012-11-27 17:52 ` Russ Dill
@ 2013-01-12 0:37 ` Mike Turquette
0 siblings, 0 replies; 5+ messages in thread
From: Mike Turquette @ 2013-01-12 0:37 UTC (permalink / raw)
To: Russ Dill, Mike Turquette; +Cc: linux-omap, Linux ARM Kernel List, linux-kernel
Quoting Russ Dill (2012-11-27 09:52:50)
> On Mon, Nov 26, 2012 at 11:57 AM, Mike Turquette <mturquette@ti.com> wrote:
> >
> > Quoting Russ Dill (2012-11-26 11:20:09)
> > > The helper functions that access the opaque struct clk should
> > > not be marked inline since they are contained in clk.c, but expected
> > > to be used by other compilation units. This causes compile errors
> > > under gcc-4.7
> > >
> > > In file included from arch/arm/mach-omap2/clockdomain.c:25:0:
> > > arch/arm/mach-omap2/clockdomain.c: In function ‘clkdm_clk_disable’:
> > > include/linux/clk-provider.h:338:12: error: inlining failed in call to always_inline ‘__clk_get_enable_count’: function body not available
> > > arch/arm/mach-omap2/clockdomain.c:1001:28: error: called from here
> > > make[1]: *** [arch/arm/mach-omap2/clockdomain.o] Error 1
> > > make: *** [arch/arm/mach-omap2] Error 2
> > >
> >
> > Hi Russ,
> >
> > A fix for this was merged into rc7. See 93532c8a, "clk: remove inline
> > usage from clk-provider.h".
> >
> > Regardless, I'm still considering this patch. I've heard many times
> > that we should trust the compiler to optimize for us and some folks look
> > down on inlining in general. If anyone has an opinion on removing
> > inlines from the common clk core then please do speak up.
> >
> > Russ, can you update to the latest rc and verify if that fix is enough
> > for you?
>
> Yes, that commit fixes the compile issue too. I'd go with the more
> complete removal of inlines though. If you have a declaration in a
> header, it makes no sense to call it inline in the .c
>
Hi Russ,
I've taken this into clk-next.
Thanks,
Mike
> > Regards,
> > Mike
> >
> > >
> > > Signed-off-by: Russ Dill <Russ.Dill@ti.com>
> > > ---
> > > drivers/clk/clk.c | 14 +++++++-------
> > > include/linux/clk-provider.h | 4 ++--
> > > 2 files changed, 9 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > > index 56e4495e..ed01746 100644
> > > --- a/drivers/clk/clk.c
> > > +++ b/drivers/clk/clk.c
> > > @@ -249,32 +249,32 @@ late_initcall(clk_disable_unused);
> > >
> > > /*** helper functions ***/
> > >
> > > -inline const char *__clk_get_name(struct clk *clk)
> > > +const char *__clk_get_name(struct clk *clk)
> > > {
> > > return !clk ? NULL : clk->name;
> > > }
> > >
> > > -inline struct clk_hw *__clk_get_hw(struct clk *clk)
> > > +struct clk_hw *__clk_get_hw(struct clk *clk)
> > > {
> > > return !clk ? NULL : clk->hw;
> > > }
> > >
> > > -inline u8 __clk_get_num_parents(struct clk *clk)
> > > +u8 __clk_get_num_parents(struct clk *clk)
> > > {
> > > return !clk ? -EINVAL : clk->num_parents;
> > > }
> > >
> > > -inline struct clk *__clk_get_parent(struct clk *clk)
> > > +struct clk *__clk_get_parent(struct clk *clk)
> > > {
> > > return !clk ? NULL : clk->parent;
> > > }
> > >
> > > -inline int __clk_get_enable_count(struct clk *clk)
> > > +int __clk_get_enable_count(struct clk *clk)
> > > {
> > > return !clk ? -EINVAL : clk->enable_count;
> > > }
> > >
> > > -inline int __clk_get_prepare_count(struct clk *clk)
> > > +int __clk_get_prepare_count(struct clk *clk)
> > > {
> > > return !clk ? -EINVAL : clk->prepare_count;
> > > }
> > > @@ -300,7 +300,7 @@ out:
> > > return ret;
> > > }
> > >
> > > -inline unsigned long __clk_get_flags(struct clk *clk)
> > > +unsigned long __clk_get_flags(struct clk *clk)
> > > {
> > > return !clk ? -EINVAL : clk->flags;
> > > }
> > > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> > > index c127315..f9f5e9e 100644
> > > --- a/include/linux/clk-provider.h
> > > +++ b/include/linux/clk-provider.h
> > > @@ -335,8 +335,8 @@ const char *__clk_get_name(struct clk *clk);
> > > struct clk_hw *__clk_get_hw(struct clk *clk);
> > > u8 __clk_get_num_parents(struct clk *clk);
> > > struct clk *__clk_get_parent(struct clk *clk);
> > > -inline int __clk_get_enable_count(struct clk *clk);
> > > -inline int __clk_get_prepare_count(struct clk *clk);
> > > +int __clk_get_enable_count(struct clk *clk);
> > > +int __clk_get_prepare_count(struct clk *clk);
> > > unsigned long __clk_get_rate(struct clk *clk);
> > > unsigned long __clk_get_flags(struct clk *clk);
> > > int __clk_is_enabled(struct clk *clk);
> > > --
> > > 1.8.0
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-01-12 0:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-26 19:20 [PATCH] Don't mark shared helper functions as inline Russ Dill
2012-11-26 19:57 ` Mike Turquette
2012-11-26 21:00 ` Mark A. Greer
2012-11-27 17:52 ` Russ Dill
2013-01-12 0:37 ` Mike Turquette
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).