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

[Winkler, Tomas]

> On Mon, Mar 20, 2017 at 1:11 PM, Winkler, Tomas
> <tomas.winkler@xxxxxxxxx> 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