Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1605193

RE: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused

Show all headers | View raw

> 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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused Arnd Bergmann <arnd@arndb.de> - 2017-03-20 10:20 +0100
  RE: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused "Winkler, Tomas" <tomas.winkler@intel.com> - 2017-03-20 13:20 +0100
    Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused Arnd Bergmann <arnd@arndb.de> - 2017-03-20 13:30 +0100
      RE: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused "Winkler, Tomas" <tomas.winkler@intel.com> - 2017-03-21 00:10 +0100
        Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-03-21 00:10 +0100
          RE: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused "Winkler, Tomas" <tomas.winkler@intel.com> - 2017-03-21 00:40 +0100
            Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused Arnd Bergmann <arnd@arndb.de> - 2017-03-21 08:40 +0100
  Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-03-22 11:30 +0100

csiph-web