staging: greybus: mark PM functions as __maybe_unused

Message ID 20170518132834.1744968-1-arnd@arndb.de
State Accepted
Commit 0687090acf0d0bee585fa0ec29cf5b1fd48cefee
Headers show

Commit Message

Arnd Bergmann May 18, 2017, 1:28 p.m.
Enabling the arche platform for compile testing showed a harmless
warning with CONFIG_PM=n:

drivers/staging/greybus/arche-platform.c:632:12: error: 'arche_platform_resume' defined but not used [-Werror=unused-function]
drivers/staging/greybus/arche-platform.c:618:12: error: 'arche_platform_suspend' defined but not used [-Werror=unused-function]

This marks the functions as __maybe_unused to shut up the warnings.

Fixes: 2eccd4aa19fc ("staging: greybus: enable compile testing of arche driver")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/staging/greybus/arche-platform.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.9.0

Comments

Johan Hovold May 18, 2017, 1:33 p.m. | #1
On Thu, May 18, 2017 at 03:28:00PM +0200, Arnd Bergmann wrote:
> Enabling the arche platform for compile testing showed a harmless

> warning with CONFIG_PM=n:

> 

> drivers/staging/greybus/arche-platform.c:632:12: error: 'arche_platform_resume' defined but not used [-Werror=unused-function]

> drivers/staging/greybus/arche-platform.c:618:12: error: 'arche_platform_suspend' defined but not used [-Werror=unused-function]

> 

> This marks the functions as __maybe_unused to shut up the warnings.

> 

> Fixes: 2eccd4aa19fc ("staging: greybus: enable compile testing of arche driver")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>


Acked-by: Johan Hovold <johan@kernel.org>


Thanks for fixing this.

Johan
Viresh Kumar May 18, 2017, 2:18 p.m. | #2
On 18-05-17, 15:28, Arnd Bergmann wrote:
> Enabling the arche platform for compile testing showed a harmless

> warning with CONFIG_PM=n:

> 

> drivers/staging/greybus/arche-platform.c:632:12: error: 'arche_platform_resume' defined but not used [-Werror=unused-function]

> drivers/staging/greybus/arche-platform.c:618:12: error: 'arche_platform_suspend' defined but not used [-Werror=unused-function]

> 

> This marks the functions as __maybe_unused to shut up the warnings.

> 

> Fixes: 2eccd4aa19fc ("staging: greybus: enable compile testing of arche driver")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

>  drivers/staging/greybus/arche-platform.c | 4 ++--

>  1 file changed, 2 insertions(+), 2 deletions(-)

> 

> diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c

> index 5bce5e039596..eced2d26467b 100644

> --- a/drivers/staging/greybus/arche-platform.c

> +++ b/drivers/staging/greybus/arche-platform.c

> @@ -615,7 +615,7 @@ static int arche_platform_remove(struct platform_device *pdev)

>  	return 0;

>  }

>  

> -static int arche_platform_suspend(struct device *dev)

> +static __maybe_unused int arche_platform_suspend(struct device *dev)

>  {

>  	/*

>  	 * If timing profile premits, we may shutdown bridge

> @@ -629,7 +629,7 @@ static int arche_platform_suspend(struct device *dev)

>  	return 0;

>  }

>  

> -static int arche_platform_resume(struct device *dev)

> +static __maybe_unused int arche_platform_resume(struct device *dev)

>  {

>  	/*

>  	 * Atleast for ES2 we have to meet the delay requirement between


Is __maybe_unused the more preferred way than putting these routines
under CONFIG_PM_SLEEP ifdef ?

-- 
viresh
Arnd Bergmann May 18, 2017, 2:51 p.m. | #3
On Thu, May 18, 2017 at 4:18 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 18-05-17, 15:28, Arnd Bergmann wrote:

>> Enabling the arche platform for compile testing showed a harmless

>> warning with CONFIG_PM=n:

>>

>> drivers/staging/greybus/arche-platform.c:632:12: error: 'arche_platform_resume' defined but not used [-Werror=unused-function]

>> drivers/staging/greybus/arche-platform.c:618:12: error: 'arche_platform_suspend' defined but not used [-Werror=unused-function]

>>

>> This marks the functions as __maybe_unused to shut up the warnings.

>>

>> Fixes: 2eccd4aa19fc ("staging: greybus: enable compile testing of arche driver")

>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

>> ---

>>  drivers/staging/greybus/arche-platform.c | 4 ++--

>>  1 file changed, 2 insertions(+), 2 deletions(-)

>>

>> diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c

>> index 5bce5e039596..eced2d26467b 100644

>> --- a/drivers/staging/greybus/arche-platform.c

>> +++ b/drivers/staging/greybus/arche-platform.c

>> @@ -615,7 +615,7 @@ static int arche_platform_remove(struct platform_device *pdev)

>>       return 0;

>>  }

>>

>> -static int arche_platform_suspend(struct device *dev)

>> +static __maybe_unused int arche_platform_suspend(struct device *dev)

>>  {

>>       /*

>>        * If timing profile premits, we may shutdown bridge

>> @@ -629,7 +629,7 @@ static int arche_platform_suspend(struct device *dev)

>>       return 0;

>>  }

>>

>> -static int arche_platform_resume(struct device *dev)

>> +static __maybe_unused int arche_platform_resume(struct device *dev)

>>  {

>>       /*

>>        * Atleast for ES2 we have to meet the delay requirement between

>

> Is __maybe_unused the more preferred way than putting these routines

> under CONFIG_PM_SLEEP ifdef ?


I find that a lot of users get the #ifdef wrong, either using the wrong
macro (CONFIG_PM vs CONFIG_PM_SLEEP) or not using the right
set of functions (e.g. calling a function only from the suspend handler).

The __maybe_unused annotation avoids both problems and also gives
better build time coverage, so that's what I tend to use.

     Arnd
Viresh Kumar May 19, 2017, 3:40 a.m. | #4
On 18-05-17, 16:51, Arnd Bergmann wrote:
> I find that a lot of users get the #ifdef wrong, either using the wrong

> macro (CONFIG_PM vs CONFIG_PM_SLEEP) or not using the right

> set of functions (e.g. calling a function only from the suspend handler).

> 

> The __maybe_unused annotation avoids both problems and also gives

> better build time coverage, so that's what I tend to use.


Thanks for the explanation Arnd. I hope these unused routines will not
be part of the binary that gets generated. Right?

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>


-- 
viresh
Arnd Bergmann May 19, 2017, 6:57 a.m. | #5
On Fri, May 19, 2017 at 5:40 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 18-05-17, 16:51, Arnd Bergmann wrote:

>> I find that a lot of users get the #ifdef wrong, either using the wrong

>> macro (CONFIG_PM vs CONFIG_PM_SLEEP) or not using the right

>> set of functions (e.g. calling a function only from the suspend handler).

>>

>> The __maybe_unused annotation avoids both problems and also gives

>> better build time coverage, so that's what I tend to use.

>

> Thanks for the explanation Arnd. I hope these unused routines will not

> be part of the binary that gets generated. Right?


Correct. Ancient compilers (gcc-4.1) had a bug where a function would
still be part of the binary if the only reference to it was from a function
pointer that got dropped through dead code elimination, but that is not
the case here, and those old compilers are not used in real life any more
either.

       Arnd

Patch

diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c
index 5bce5e039596..eced2d26467b 100644
--- a/drivers/staging/greybus/arche-platform.c
+++ b/drivers/staging/greybus/arche-platform.c
@@ -615,7 +615,7 @@  static int arche_platform_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static int arche_platform_suspend(struct device *dev)
+static __maybe_unused int arche_platform_suspend(struct device *dev)
 {
 	/*
 	 * If timing profile premits, we may shutdown bridge
@@ -629,7 +629,7 @@  static int arche_platform_suspend(struct device *dev)
 	return 0;
 }
 
-static int arche_platform_resume(struct device *dev)
+static __maybe_unused int arche_platform_resume(struct device *dev)
 {
 	/*
 	 * Atleast for ES2 we have to meet the delay requirement between