All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused
@ 2017-03-20  9:17 Arnd Bergmann
  2017-03-20 12:11   ` Winkler, Tomas
  2017-03-22 10:21   ` Jarkko Sakkinen
  0 siblings, 2 replies; 15+ messages in thread
From: Arnd Bergmann @ 2017-03-20  9:17 UTC (permalink / raw)
  To: Peter Huewe, Marcel Selhorst, Jarkko Sakkinen
  Cc: Arnd Bergmann, Jason Gunthorpe, Winkler, Tomas, Jerry Snitselaar,
	tpmdd-devel, linux-kernel

When CONFIG_PM_SLEEP is disabled, we get a warning about unused functions:

drivers/char/tpm/tpm_crb.c:551:12: error: 'crb_pm_resume' defined but not used [-Werror=unused-function]
drivers/char/tpm/tpm_crb.c:540:12: error: 'crb_pm_suspend' defined but not used [-Werror=unused-function]

We could solve this with more sophistated #ifdefs, but a simpler and safer
way is to just mark them as __maybe_unused.

Fixes: 848efcfb560c ("tpm/tpm_crb: enter the low power state upon device suspend")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/char/tpm/tpm_crb.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 1dfc37e33c02..15f1118982a6 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -519,8 +519,7 @@ static int crb_acpi_remove(struct acpi_device *device)
 	return 0;
 }
 
-#ifdef CONFIG_PM
-static int crb_pm_runtime_suspend(struct device *dev)
+static __maybe_unused int crb_pm_runtime_suspend(struct device *dev)
 {
 	struct tpm_chip *chip = dev_get_drvdata(dev);
 	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
@@ -528,7 +527,7 @@ static int crb_pm_runtime_suspend(struct device *dev)
 	return crb_go_idle(dev, priv);
 }
 
-static int crb_pm_runtime_resume(struct device *dev)
+static __maybe_unused int crb_pm_runtime_resume(struct device *dev)
 {
 	struct tpm_chip *chip = dev_get_drvdata(dev);
 	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
@@ -536,7 +535,7 @@ static int crb_pm_runtime_resume(struct device *dev)
 	return crb_cmd_ready(dev, priv);
 }
 
-static int crb_pm_suspend(struct device *dev)
+static __maybe_unused int crb_pm_suspend(struct device *dev)
 {
 	int ret;
 
@@ -547,7 +546,7 @@ static int crb_pm_suspend(struct device *dev)
 	return crb_pm_runtime_suspend(dev);
 }
 
-static int crb_pm_resume(struct device *dev)
+static __maybe_unused int crb_pm_resume(struct device *dev)
 {
 	int ret;
 
@@ -558,8 +557,6 @@ static int crb_pm_resume(struct device *dev)
 	return tpm_pm_resume(dev);
 }
 
-#endif /* CONFIG_PM */
-
 static const struct dev_pm_ops crb_pm = {
 	SET_SYSTEM_SLEEP_PM_OPS(crb_pm_suspend, crb_pm_resume)
 	SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend, crb_pm_runtime_resume, NULL)
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* RE: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused
@ 2017-03-20 12:11   ` Winkler, Tomas
  0 siblings, 0 replies; 15+ messages in thread
From: Winkler, Tomas @ 2017-03-20 12:11 UTC (permalink / raw)
  To: Arnd Bergmann, Peter Huewe, Marcel Selhorst, Jarkko Sakkinen
  Cc: Jason Gunthorpe, Jerry Snitselaar, tpmdd-devel, linux-kernel

> 
> When CONFIG_PM_SLEEP is disabled, we get a warning about unused
> functions:
> 
> drivers/char/tpm/tpm_crb.c:551:12: error: 'crb_pm_resume' defined but not
> used [-Werror=unused-function]
> drivers/char/tpm/tpm_crb.c:540:12: error: 'crb_pm_suspend' defined but not
> used [-Werror=unused-function]
>
Note that the runtime_pm functions are not affected by this issue the macro SET_RUNTIME_PM_OPS
is under CONFIG_PM. This patch does more than described. 

> We could solve this with more sophistated #ifdefs, but a simpler and safer way
> is to just mark them as __maybe_unused.

> 
> Fixes: 848efcfb560c ("tpm/tpm_crb: enter the low power state upon device
> suspend")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/char/tpm/tpm_crb.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c index
> 1dfc37e33c02..15f1118982a6 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -519,8 +519,7 @@ static int crb_acpi_remove(struct acpi_device *device)
>  	return 0;
>  }
> 
> -#ifdef CONFIG_PM
> -static int crb_pm_runtime_suspend(struct device *dev)
> +static __maybe_unused int crb_pm_runtime_suspend(struct device *dev)
>  {
>  	struct tpm_chip *chip = dev_get_drvdata(dev);
>  	struct crb_priv *priv = dev_get_drvdata(&chip->dev); @@ -528,7
> +527,7 @@ static int crb_pm_runtime_suspend(struct device *dev)
>  	return crb_go_idle(dev, priv);
>  }
> 
> -static int crb_pm_runtime_resume(struct device *dev)
> +static __maybe_unused int crb_pm_runtime_resume(struct device *dev)
>  {
>  	struct tpm_chip *chip = dev_get_drvdata(dev);
>  	struct crb_priv *priv = dev_get_drvdata(&chip->dev); @@ -536,7
> +535,7 @@ static int crb_pm_runtime_resume(struct device *dev)
>  	return crb_cmd_ready(dev, priv);
>  }
> 
> -static int crb_pm_suspend(struct device *dev)
> +static __maybe_unused int crb_pm_suspend(struct device *dev)
>  {
>  	int ret;
> 
> @@ -547,7 +546,7 @@ static int crb_pm_suspend(struct device *dev)
>  	return crb_pm_runtime_suspend(dev);
>  }

It's enough to 
#endif /* CONFIG_PM */
#ifdef CONFIG_PM_SLEEP
> -static int crb_pm_resume(struct device *dev)
> +static __maybe_unused int crb_pm_resume(struct device *dev)
>  {
>  	int ret;
> 
> @@ -558,8 +557,6 @@ static int crb_pm_resume(struct device *dev)
>  	return tpm_pm_resume(dev);
>  }
> 
> -#endif /* CONFIG_PM */
And 
#endif CONFIG_PM_SLEEP

> -
>  static const struct dev_pm_ops crb_pm = {
>  	SET_SYSTEM_SLEEP_PM_OPS(crb_pm_suspend, crb_pm_resume)
>  	SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend,
> crb_pm_runtime_resume, NULL)
> --
> 2.9.0

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused
@ 2017-03-20 12:11   ` Winkler, Tomas
  0 siblings, 0 replies; 15+ messages in thread
From: Winkler, Tomas @ 2017-03-20 12:11 UTC (permalink / raw)
  To: Arnd Bergmann, Peter Huewe, Marcel Selhorst, Jarkko Sakkinen
  Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

> 
> When CONFIG_PM_SLEEP is disabled, we get a warning about unused
> functions:
> 
> drivers/char/tpm/tpm_crb.c:551:12: error: 'crb_pm_resume' defined but not
> used [-Werror=unused-function]
> drivers/char/tpm/tpm_crb.c:540:12: error: 'crb_pm_suspend' defined but not
> used [-Werror=unused-function]
>
Note that the runtime_pm functions are not affected by this issue the macro SET_RUNTIME_PM_OPS
is under CONFIG_PM. This patch does more than described. 

> We could solve this with more sophistated #ifdefs, but a simpler and safer way
> is to just mark them as __maybe_unused.

> 
> Fixes: 848efcfb560c ("tpm/tpm_crb: enter the low power state upon device
> suspend")
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> ---
>  drivers/char/tpm/tpm_crb.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c index
> 1dfc37e33c02..15f1118982a6 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -519,8 +519,7 @@ static int crb_acpi_remove(struct acpi_device *device)
>  	return 0;
>  }
> 
> -#ifdef CONFIG_PM
> -static int crb_pm_runtime_suspend(struct device *dev)
> +static __maybe_unused int crb_pm_runtime_suspend(struct device *dev)
>  {
>  	struct tpm_chip *chip = dev_get_drvdata(dev);
>  	struct crb_priv *priv = dev_get_drvdata(&chip->dev); @@ -528,7
> +527,7 @@ static int crb_pm_runtime_suspend(struct device *dev)
>  	return crb_go_idle(dev, priv);
>  }
> 
> -static int crb_pm_runtime_resume(struct device *dev)
> +static __maybe_unused int crb_pm_runtime_resume(struct device *dev)
>  {
>  	struct tpm_chip *chip = dev_get_drvdata(dev);
>  	struct crb_priv *priv = dev_get_drvdata(&chip->dev); @@ -536,7
> +535,7 @@ static int crb_pm_runtime_resume(struct device *dev)
>  	return crb_cmd_ready(dev, priv);
>  }
> 
> -static int crb_pm_suspend(struct device *dev)
> +static __maybe_unused int crb_pm_suspend(struct device *dev)
>  {
>  	int ret;
> 
> @@ -547,7 +546,7 @@ static int crb_pm_suspend(struct device *dev)
>  	return crb_pm_runtime_suspend(dev);
>  }

It's enough to 
#endif /* CONFIG_PM */
#ifdef CONFIG_PM_SLEEP
> -static int crb_pm_resume(struct device *dev)
> +static __maybe_unused int crb_pm_resume(struct device *dev)
>  {
>  	int ret;
> 
> @@ -558,8 +557,6 @@ static int crb_pm_resume(struct device *dev)
>  	return tpm_pm_resume(dev);
>  }
> 
> -#endif /* CONFIG_PM */
And 
#endif CONFIG_PM_SLEEP

> -
>  static const struct dev_pm_ops crb_pm = {
>  	SET_SYSTEM_SLEEP_PM_OPS(crb_pm_suspend, crb_pm_resume)
>  	SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend,
> crb_pm_runtime_resume, NULL)
> --
> 2.9.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused
@ 2017-03-20 12:27     ` Arnd Bergmann
  0 siblings, 0 replies; 15+ messages in thread
From: Arnd Bergmann @ 2017-03-20 12:27 UTC (permalink / raw)
  To: Winkler, Tomas
  Cc: Peter Huewe, Marcel Selhorst, Jarkko Sakkinen, Jason Gunthorpe,
	Jerry Snitselaar, tpmdd-devel, linux-kernel

On Mon, Mar 20, 2017 at 1:11 PM, Winkler, Tomas <tomas.winkler@intel.com> wrote:
>>
>> When CONFIG_PM_SLEEP is disabled, we get a warning about unused
>> functions:
>>
>> drivers/char/tpm/tpm_crb.c:551:12: error: 'crb_pm_resume' defined but not
>> used [-Werror=unused-function]
>> drivers/char/tpm/tpm_crb.c:540:12: error: 'crb_pm_suspend' defined but not
>> used [-Werror=unused-function]
>>
> Note that the runtime_pm functions are not affected by this issue the macro
> SET_RUNTIME_PM_OPS is under CONFIG_PM. This patch does more than described.

Well, the problem is that there is an #ifdef that is wrong here as I
tried to indicate:

>> We could solve this with more sophistated #ifdefs, but a simpler and safer way
>> is to just mark them as __maybe_unused.

>> @@ -547,7 +546,7 @@ static int crb_pm_suspend(struct device *dev)
>>       return crb_pm_runtime_suspend(dev);
>>  }
>
> It's enough to
> #endif /* CONFIG_PM */
> #ifdef CONFIG_PM_SLEEP
>> -static int crb_pm_resume(struct device *dev)
>> +static __maybe_unused int crb_pm_resume(struct device *dev)
>>  {
>>       int ret;
>>
>> @@ -558,8 +557,6 @@ static int crb_pm_resume(struct device *dev)
>>       return tpm_pm_resume(dev);
>>  }
>>
>> -#endif /* CONFIG_PM */
> And
> #endif CONFIG_PM_SLEEP

This tends to cause other warnings half of the time, when both the
runtime-pm and pm-sleep variants call into another function that
becomes unused when both are disabled.

       Arnd

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused
@ 2017-03-20 12:27     ` Arnd Bergmann
  0 siblings, 0 replies; 15+ messages in thread
From: Arnd Bergmann @ 2017-03-20 12:27 UTC (permalink / raw)
  To: Winkler, Tomas
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Mon, Mar 20, 2017 at 1:11 PM, Winkler, Tomas <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>>
>> When CONFIG_PM_SLEEP is disabled, we get a warning about unused
>> functions:
>>
>> drivers/char/tpm/tpm_crb.c:551:12: error: 'crb_pm_resume' defined but not
>> used [-Werror=unused-function]
>> drivers/char/tpm/tpm_crb.c:540:12: error: 'crb_pm_suspend' defined but not
>> used [-Werror=unused-function]
>>
> Note that the runtime_pm functions are not affected by this issue the macro
> SET_RUNTIME_PM_OPS is under CONFIG_PM. This patch does more than described.

Well, the problem is that there is an #ifdef that is wrong here as I
tried to indicate:

>> We could solve this with more sophistated #ifdefs, but a simpler and safer way
>> is to just mark them as __maybe_unused.

>> @@ -547,7 +546,7 @@ static int crb_pm_suspend(struct device *dev)
>>       return crb_pm_runtime_suspend(dev);
>>  }
>
> It's enough to
> #endif /* CONFIG_PM */
> #ifdef CONFIG_PM_SLEEP
>> -static int crb_pm_resume(struct device *dev)
>> +static __maybe_unused int crb_pm_resume(struct device *dev)
>>  {
>>       int ret;
>>
>> @@ -558,8 +557,6 @@ static int crb_pm_resume(struct device *dev)
>>       return tpm_pm_resume(dev);
>>  }
>>
>> -#endif /* CONFIG_PM */
> And
> #endif CONFIG_PM_SLEEP

This tends to cause other warnings half of the time, when both the
runtime-pm and pm-sleep variants call into another function that
becomes unused when both are disabled.

       Arnd

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused
@ 2017-03-20 23:01       ` Winkler, Tomas
  0 siblings, 0 replies; 15+ messages in thread
From: Winkler, Tomas @ 2017-03-20 23:01 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Peter Huewe, Marcel Selhorst, Jarkko Sakkinen, Jason Gunthorpe,
	Jerry Snitselaar, tpmdd-devel, linux-kernel


> On Mon, Mar 20, 2017 at 1:11 PM, Winkler, Tomas
> <tomas.winkler@intel.com> wrote:
> >>
> >> When CONFIG_PM_SLEEP is disabled, we get a warning about unused
> >> functions:
> >>
> >> drivers/char/tpm/tpm_crb.c:551:12: error: 'crb_pm_resume' defined but
> >> not used [-Werror=unused-function]
> >> drivers/char/tpm/tpm_crb.c:540:12: error: 'crb_pm_suspend' defined
> >> but not used [-Werror=unused-function]
> >>
> > Note that the runtime_pm functions are not affected by this issue the
> > macro SET_RUNTIME_PM_OPS is under CONFIG_PM. This patch does more
> than described.
> 
> Well, the problem is that there is an #ifdef that is wrong here as I tried to
> indicate:

Yep, I've taken a bit easy path here and reused the runtime function inside pm callbacks, unaware of the change in the 'ifdefs'
If I use drictly go_idle/cmd_ready  functions this will unclutter it. 

> >> We could solve this with more sophistated #ifdefs, but a simpler and
> >> safer way is to just mark them as __maybe_unused.
> 
> >> @@ -547,7 +546,7 @@ static int crb_pm_suspend(struct device *dev)
> >>       return crb_pm_runtime_suspend(dev);  }
> >
> > It's enough to
> > #endif /* CONFIG_PM */
> > #ifdef CONFIG_PM_SLEEP
> >> -static int crb_pm_resume(struct device *dev)
> >> +static __maybe_unused int crb_pm_resume(struct device *dev)
> >>  {
> >>       int ret;
> >>
> >> @@ -558,8 +557,6 @@ static int crb_pm_resume(struct device *dev)
> >>       return tpm_pm_resume(dev);
> >>  }
> >>
> >> -#endif /* CONFIG_PM */
> > And
> > #endif CONFIG_PM_SLEEP
> 
> This tends to cause other warnings half of the time, when both the runtime-
> pm and pm-sleep variants call into another function that becomes unused
> when both are disabled.

Then usually such functions  should be also under 'ifdef', but I understand your point in
some cases it might be not so straight forward.
The only reason against marking the function __maybe_unsed is  maybe the binary size.

I believe that  in this case the #ifdefs can be done correctly quite easily, 
but now I'm not against your solution as well, just maybe put some of this info to the commit message. 

Thanks
Tomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused
@ 2017-03-20 23:01       ` Winkler, Tomas
  0 siblings, 0 replies; 15+ messages in thread
From: Winkler, Tomas @ 2017-03-20 23:01 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f


> On Mon, Mar 20, 2017 at 1:11 PM, Winkler, Tomas
> <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> >>
> >> When CONFIG_PM_SLEEP is disabled, we get a warning about unused
> >> functions:
> >>
> >> drivers/char/tpm/tpm_crb.c:551:12: error: 'crb_pm_resume' defined but
> >> not used [-Werror=unused-function]
> >> drivers/char/tpm/tpm_crb.c:540:12: error: 'crb_pm_suspend' defined
> >> but not used [-Werror=unused-function]
> >>
> > Note that the runtime_pm functions are not affected by this issue the
> > macro SET_RUNTIME_PM_OPS is under CONFIG_PM. This patch does more
> than described.
> 
> Well, the problem is that there is an #ifdef that is wrong here as I tried to
> indicate:

Yep, I've taken a bit easy path here and reused the runtime function inside pm callbacks, unaware of the change in the 'ifdefs'
If I use drictly go_idle/cmd_ready  functions this will unclutter it. 

> >> We could solve this with more sophistated #ifdefs, but a simpler and
> >> safer way is to just mark them as __maybe_unused.
> 
> >> @@ -547,7 +546,7 @@ static int crb_pm_suspend(struct device *dev)
> >>       return crb_pm_runtime_suspend(dev);  }
> >
> > It's enough to
> > #endif /* CONFIG_PM */
> > #ifdef CONFIG_PM_SLEEP
> >> -static int crb_pm_resume(struct device *dev)
> >> +static __maybe_unused int crb_pm_resume(struct device *dev)
> >>  {
> >>       int ret;
> >>
> >> @@ -558,8 +557,6 @@ static int crb_pm_resume(struct device *dev)
> >>       return tpm_pm_resume(dev);
> >>  }
> >>
> >> -#endif /* CONFIG_PM */
> > And
> > #endif CONFIG_PM_SLEEP
> 
> This tends to cause other warnings half of the time, when both the runtime-
> pm and pm-sleep variants call into another function that becomes unused
> when both are disabled.

Then usually such functions  should be also under 'ifdef', but I understand your point in
some cases it might be not so straight forward.
The only reason against marking the function __maybe_unsed is  maybe the binary size.

I believe that  in this case the #ifdefs can be done correctly quite easily, 
but now I'm not against your solution as well, just maybe put some of this info to the commit message. 

Thanks
Tomas



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused
@ 2017-03-20 23:04         ` Jason Gunthorpe
  0 siblings, 0 replies; 15+ messages in thread
From: Jason Gunthorpe @ 2017-03-20 23:04 UTC (permalink / raw)
  To: Winkler, Tomas
  Cc: Arnd Bergmann, Peter Huewe, Marcel Selhorst, Jarkko Sakkinen,
	Jerry Snitselaar, tpmdd-devel, linux-kernel

On Mon, Mar 20, 2017 at 11:01:36PM +0000, Winkler, Tomas wrote:

> I believe that in this case the #ifdefs can be done correctly quite
> easily, but now I'm not against your solution as well, just maybe
> put some of this info to the commit message.

I perfer fewer ifdefs, it makes it more maintainable..

The compiler will remove unused static functions.

Jason

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused
@ 2017-03-20 23:04         ` Jason Gunthorpe
  0 siblings, 0 replies; 15+ messages in thread
From: Jason Gunthorpe @ 2017-03-20 23:04 UTC (permalink / raw)
  To: Winkler, Tomas
  Cc: Arnd Bergmann, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Mon, Mar 20, 2017 at 11:01:36PM +0000, Winkler, Tomas wrote:

> I believe that in this case the #ifdefs can be done correctly quite
> easily, but now I'm not against your solution as well, just maybe
> put some of this info to the commit message.

I perfer fewer ifdefs, it makes it more maintainable..

The compiler will remove unused static functions.

Jason

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused
@ 2017-03-20 23:35           ` Winkler, Tomas
  0 siblings, 0 replies; 15+ messages in thread
From: Winkler, Tomas @ 2017-03-20 23:35 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Arnd Bergmann, Peter Huewe, Marcel Selhorst, Jarkko Sakkinen,
	Jerry Snitselaar, tpmdd-devel, linux-kernel


> 
> On Mon, Mar 20, 2017 at 11:01:36PM +0000, Winkler, Tomas wrote:
> 
> > I believe that in this case the #ifdefs can be done correctly quite
> > easily, but now I'm not against your solution as well, just maybe put
> > some of this info to the commit message.
> 
> I perfer fewer ifdefs, it makes it more maintainable..

Sure,
> 
> The compiler will remove unused static functions.

I'm not sure if this goes away w/o --gc-sections, but it might. 
Will check, didn't looked at that for a while. 

Thanks
Tomas 

Tomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused
@ 2017-03-20 23:35           ` Winkler, Tomas
  0 siblings, 0 replies; 15+ messages in thread
From: Winkler, Tomas @ 2017-03-20 23:35 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Arnd Bergmann, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f


> 
> On Mon, Mar 20, 2017 at 11:01:36PM +0000, Winkler, Tomas wrote:
> 
> > I believe that in this case the #ifdefs can be done correctly quite
> > easily, but now I'm not against your solution as well, just maybe put
> > some of this info to the commit message.
> 
> I perfer fewer ifdefs, it makes it more maintainable..

Sure,
> 
> The compiler will remove unused static functions.

I'm not sure if this goes away w/o --gc-sections, but it might. 
Will check, didn't looked at that for a while. 

Thanks
Tomas 

Tomas


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused
@ 2017-03-21  7:37             ` Arnd Bergmann
  0 siblings, 0 replies; 15+ messages in thread
From: Arnd Bergmann @ 2017-03-21  7:37 UTC (permalink / raw)
  To: Winkler, Tomas
  Cc: Jason Gunthorpe, Peter Huewe, Marcel Selhorst, Jarkko Sakkinen,
	Jerry Snitselaar, tpmdd-devel, linux-kernel

On Tue, Mar 21, 2017 at 12:35 AM, Winkler, Tomas
<tomas.winkler@intel.com> wrote:
>
>>
>> On Mon, Mar 20, 2017 at 11:01:36PM +0000, Winkler, Tomas wrote:
>>
>> > I believe that in this case the #ifdefs can be done correctly quite
>> > easily, but now I'm not against your solution as well, just maybe put
>> > some of this info to the commit message.
>>
>> I perfer fewer ifdefs, it makes it more maintainable..
>
> Sure,
>>
>> The compiler will remove unused static functions.
>
> I'm not sure if this goes away w/o --gc-sections, but it might.
> Will check, didn't looked at that for a while.

gcc-4.1 had a bug where code it failed to eliminate a dead function
if it was referenced through a function pointer in another unused
static function, but it would work correctly in this case (obviously
unused code) and compiler that people actually use don't have
this problem. Note that the kernel depends on dead code elimination
to work correctly in a lot of places, it wouldn't build at all if that
was broken.

      Arnd

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused
@ 2017-03-21  7:37             ` Arnd Bergmann
  0 siblings, 0 replies; 15+ messages in thread
From: Arnd Bergmann @ 2017-03-21  7:37 UTC (permalink / raw)
  To: Winkler, Tomas
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Tue, Mar 21, 2017 at 12:35 AM, Winkler, Tomas
<tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>
>>
>> On Mon, Mar 20, 2017 at 11:01:36PM +0000, Winkler, Tomas wrote:
>>
>> > I believe that in this case the #ifdefs can be done correctly quite
>> > easily, but now I'm not against your solution as well, just maybe put
>> > some of this info to the commit message.
>>
>> I perfer fewer ifdefs, it makes it more maintainable..
>
> Sure,
>>
>> The compiler will remove unused static functions.
>
> I'm not sure if this goes away w/o --gc-sections, but it might.
> Will check, didn't looked at that for a while.

gcc-4.1 had a bug where code it failed to eliminate a dead function
if it was referenced through a function pointer in another unused
static function, but it would work correctly in this case (obviously
unused code) and compiler that people actually use don't have
this problem. Note that the kernel depends on dead code elimination
to work correctly in a lot of places, it wouldn't build at all if that
was broken.

      Arnd

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused
@ 2017-03-22 10:21   ` Jarkko Sakkinen
  0 siblings, 0 replies; 15+ messages in thread
From: Jarkko Sakkinen @ 2017-03-22 10:21 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Peter Huewe, Marcel Selhorst, Jason Gunthorpe, Winkler, Tomas,
	Jerry Snitselaar, tpmdd-devel, linux-kernel

On Mon, Mar 20, 2017 at 10:17:19AM +0100, Arnd Bergmann wrote:
> When CONFIG_PM_SLEEP is disabled, we get a warning about unused functions:
> 
> drivers/char/tpm/tpm_crb.c:551:12: error: 'crb_pm_resume' defined but not used [-Werror=unused-function]
> drivers/char/tpm/tpm_crb.c:540:12: error: 'crb_pm_suspend' defined but not used [-Werror=unused-function]
> 
> We could solve this with more sophistated #ifdefs, but a simpler and safer
> way is to just mark them as __maybe_unused.
> 
> Fixes: 848efcfb560c ("tpm/tpm_crb: enter the low power state upon device suspend")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Hi, I somehow missed this and applied very similar patch. Sorry about
that.

/Jarkko

> ---
>  drivers/char/tpm/tpm_crb.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 1dfc37e33c02..15f1118982a6 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -519,8 +519,7 @@ static int crb_acpi_remove(struct acpi_device *device)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_PM
> -static int crb_pm_runtime_suspend(struct device *dev)
> +static __maybe_unused int crb_pm_runtime_suspend(struct device *dev)
>  {
>  	struct tpm_chip *chip = dev_get_drvdata(dev);
>  	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> @@ -528,7 +527,7 @@ static int crb_pm_runtime_suspend(struct device *dev)
>  	return crb_go_idle(dev, priv);
>  }
>  
> -static int crb_pm_runtime_resume(struct device *dev)
> +static __maybe_unused int crb_pm_runtime_resume(struct device *dev)
>  {
>  	struct tpm_chip *chip = dev_get_drvdata(dev);
>  	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> @@ -536,7 +535,7 @@ static int crb_pm_runtime_resume(struct device *dev)
>  	return crb_cmd_ready(dev, priv);
>  }
>  
> -static int crb_pm_suspend(struct device *dev)
> +static __maybe_unused int crb_pm_suspend(struct device *dev)
>  {
>  	int ret;
>  
> @@ -547,7 +546,7 @@ static int crb_pm_suspend(struct device *dev)
>  	return crb_pm_runtime_suspend(dev);
>  }
>  
> -static int crb_pm_resume(struct device *dev)
> +static __maybe_unused int crb_pm_resume(struct device *dev)
>  {
>  	int ret;
>  
> @@ -558,8 +557,6 @@ static int crb_pm_resume(struct device *dev)
>  	return tpm_pm_resume(dev);
>  }
>  
> -#endif /* CONFIG_PM */
> -
>  static const struct dev_pm_ops crb_pm = {
>  	SET_SYSTEM_SLEEP_PM_OPS(crb_pm_suspend, crb_pm_resume)
>  	SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend, crb_pm_runtime_resume, NULL)
> -- 
> 2.9.0
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused
@ 2017-03-22 10:21   ` Jarkko Sakkinen
  0 siblings, 0 replies; 15+ messages in thread
From: Jarkko Sakkinen @ 2017-03-22 10:21 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Mon, Mar 20, 2017 at 10:17:19AM +0100, Arnd Bergmann wrote:
> When CONFIG_PM_SLEEP is disabled, we get a warning about unused functions:
> 
> drivers/char/tpm/tpm_crb.c:551:12: error: 'crb_pm_resume' defined but not used [-Werror=unused-function]
> drivers/char/tpm/tpm_crb.c:540:12: error: 'crb_pm_suspend' defined but not used [-Werror=unused-function]
> 
> We could solve this with more sophistated #ifdefs, but a simpler and safer
> way is to just mark them as __maybe_unused.
> 
> Fixes: 848efcfb560c ("tpm/tpm_crb: enter the low power state upon device suspend")
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>

Hi, I somehow missed this and applied very similar patch. Sorry about
that.

/Jarkko

> ---
>  drivers/char/tpm/tpm_crb.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 1dfc37e33c02..15f1118982a6 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -519,8 +519,7 @@ static int crb_acpi_remove(struct acpi_device *device)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_PM
> -static int crb_pm_runtime_suspend(struct device *dev)
> +static __maybe_unused int crb_pm_runtime_suspend(struct device *dev)
>  {
>  	struct tpm_chip *chip = dev_get_drvdata(dev);
>  	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> @@ -528,7 +527,7 @@ static int crb_pm_runtime_suspend(struct device *dev)
>  	return crb_go_idle(dev, priv);
>  }
>  
> -static int crb_pm_runtime_resume(struct device *dev)
> +static __maybe_unused int crb_pm_runtime_resume(struct device *dev)
>  {
>  	struct tpm_chip *chip = dev_get_drvdata(dev);
>  	struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> @@ -536,7 +535,7 @@ static int crb_pm_runtime_resume(struct device *dev)
>  	return crb_cmd_ready(dev, priv);
>  }
>  
> -static int crb_pm_suspend(struct device *dev)
> +static __maybe_unused int crb_pm_suspend(struct device *dev)
>  {
>  	int ret;
>  
> @@ -547,7 +546,7 @@ static int crb_pm_suspend(struct device *dev)
>  	return crb_pm_runtime_suspend(dev);
>  }
>  
> -static int crb_pm_resume(struct device *dev)
> +static __maybe_unused int crb_pm_resume(struct device *dev)
>  {
>  	int ret;
>  
> @@ -558,8 +557,6 @@ static int crb_pm_resume(struct device *dev)
>  	return tpm_pm_resume(dev);
>  }
>  
> -#endif /* CONFIG_PM */
> -
>  static const struct dev_pm_ops crb_pm = {
>  	SET_SYSTEM_SLEEP_PM_OPS(crb_pm_suspend, crb_pm_resume)
>  	SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend, crb_pm_runtime_resume, NULL)
> -- 
> 2.9.0
> 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-03-22 10:21 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-20  9:17 [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused Arnd Bergmann
2017-03-20 12:11 ` Winkler, Tomas
2017-03-20 12:11   ` Winkler, Tomas
2017-03-20 12:27   ` Arnd Bergmann
2017-03-20 12:27     ` Arnd Bergmann
2017-03-20 23:01     ` Winkler, Tomas
2017-03-20 23:01       ` Winkler, Tomas
2017-03-20 23:04       ` Jason Gunthorpe
2017-03-20 23:04         ` Jason Gunthorpe
2017-03-20 23:35         ` Winkler, Tomas
2017-03-20 23:35           ` Winkler, Tomas
2017-03-21  7:37           ` Arnd Bergmann
2017-03-21  7:37             ` Arnd Bergmann
2017-03-22 10:21 ` Jarkko Sakkinen
2017-03-22 10:21   ` Jarkko Sakkinen

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.