| Message ID | 1456934350-1389172-7-git-send-email-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 02, 2016 at 04:58:58PM +0100, Arnd Bergmann wrote: > The sirf dma driver uses #ifdef to check for CONFIG_PM_SLEEP > for its suspend/resume code but then has no #ifdef for the > respective runtime PM code, so we get a warning if CONFIG_PM > is disabled altogether: > > drivers/dma/sirf-dma.c:1000:12: error: 'sirfsoc_dma_runtime_resume' defined but not used [-Werror=unused-function] > > This removes the existing #ifdef and instead uses __maybe_unused > annotations for all four functions to let the compiler know it > can silently drop the function definition. Hi Arnd, Rather than telling compiler that this maybe used why not add ifdef for it's suspend/resume as well, what are the demerits of that approach?
On Thursday 03 March 2016 09:17:31 Vinod Koul wrote: > On Wed, Mar 02, 2016 at 04:58:58PM +0100, Arnd Bergmann wrote: > > The sirf dma driver uses #ifdef to check for CONFIG_PM_SLEEP > > for its suspend/resume code but then has no #ifdef for the > > respective runtime PM code, so we get a warning if CONFIG_PM > > is disabled altogether: > > > > drivers/dma/sirf-dma.c:1000:12: error: 'sirfsoc_dma_runtime_resume' defined but not used [-Werror=unused-function] > > > > This removes the existing #ifdef and instead uses __maybe_unused > > annotations for all four functions to let the compiler know it > > can silently drop the function definition. > > Hi Arnd, > > Rather than telling compiler that this maybe used why not add ifdef for it's > suspend/resume as well, what are the demerits of that approach? > As I tried to explain in the cover letter, everyone gets the #ifdef wrong, and the __maybe_unused annotation is harder to get wrong here. This particular driver illustrates that well: sirfsoc_dma_remove() calls sirfsoc_dma_runtime_suspend(), so we must hide the resume function, but not suspend, and that is counterintuitive. Other drivers have other problems, e.g. functions that get called only from within the sections under an #ifdef, and then those need the same #ifdef added, which gets even more complicated when you have both runtime-pm and suspend support. Arnd
On Thu, Mar 03, 2016 at 01:33:17PM +0100, Arnd Bergmann wrote: > On Thursday 03 March 2016 09:17:31 Vinod Koul wrote: > > On Wed, Mar 02, 2016 at 04:58:58PM +0100, Arnd Bergmann wrote: > > > The sirf dma driver uses #ifdef to check for CONFIG_PM_SLEEP > > > for its suspend/resume code but then has no #ifdef for the > > > respective runtime PM code, so we get a warning if CONFIG_PM > > > is disabled altogether: > > > > > > drivers/dma/sirf-dma.c:1000:12: error: 'sirfsoc_dma_runtime_resume' defined but not used [-Werror=unused-function] > > > > > > This removes the existing #ifdef and instead uses __maybe_unused > > > annotations for all four functions to let the compiler know it > > > can silently drop the function definition. > > > > Hi Arnd, > > > > Rather than telling compiler that this maybe used why not add ifdef for it's > > suspend/resume as well, what are the demerits of that approach? > > > > As I tried to explain in the cover letter, everyone gets the #ifdef > wrong, and the __maybe_unused annotation is harder to get wrong here. > > This particular driver illustrates that well: sirfsoc_dma_remove() > calls sirfsoc_dma_runtime_suspend(), so we must hide the > resume function, but not suspend, and that is counterintuitive. > > Other drivers have other problems, e.g. functions that get called > only from within the sections under an #ifdef, and then those > need the same #ifdef added, which gets even more complicated when > you have both runtime-pm and suspend support. Thanks, applied now after fixing subsystem name
diff --git a/drivers/dma/sirf-dma.c b/drivers/dma/sirf-dma.c index 22ea2419ee56..e48350e65089 100644 --- a/drivers/dma/sirf-dma.c +++ b/drivers/dma/sirf-dma.c @@ -989,7 +989,7 @@ static int sirfsoc_dma_remove(struct platform_device *op) return 0; } -static int sirfsoc_dma_runtime_suspend(struct device *dev) +static int __maybe_unused sirfsoc_dma_runtime_suspend(struct device *dev) { struct sirfsoc_dma *sdma = dev_get_drvdata(dev); @@ -997,7 +997,7 @@ static int sirfsoc_dma_runtime_suspend(struct device *dev) return 0; } -static int sirfsoc_dma_runtime_resume(struct device *dev) +static int __maybe_unused sirfsoc_dma_runtime_resume(struct device *dev) { struct sirfsoc_dma *sdma = dev_get_drvdata(dev); int ret; @@ -1010,8 +1010,7 @@ static int sirfsoc_dma_runtime_resume(struct device *dev) return 0; } -#ifdef CONFIG_PM_SLEEP -static int sirfsoc_dma_pm_suspend(struct device *dev) +static int __maybe_unused sirfsoc_dma_pm_suspend(struct device *dev) { struct sirfsoc_dma *sdma = dev_get_drvdata(dev); struct sirfsoc_dma_regs *save = &sdma->regs_save; @@ -1062,7 +1061,7 @@ static int sirfsoc_dma_pm_suspend(struct device *dev) return 0; } -static int sirfsoc_dma_pm_resume(struct device *dev) +static int __maybe_unused sirfsoc_dma_pm_resume(struct device *dev) { struct sirfsoc_dma *sdma = dev_get_drvdata(dev); struct sirfsoc_dma_regs *save = &sdma->regs_save; @@ -1121,7 +1120,6 @@ static int sirfsoc_dma_pm_resume(struct device *dev) return 0; } -#endif static const struct dev_pm_ops sirfsoc_dma_pm_ops = { SET_RUNTIME_PM_OPS(sirfsoc_dma_runtime_suspend, sirfsoc_dma_runtime_resume, NULL)
The sirf dma driver uses #ifdef to check for CONFIG_PM_SLEEP for its suspend/resume code but then has no #ifdef for the respective runtime PM code, so we get a warning if CONFIG_PM is disabled altogether: drivers/dma/sirf-dma.c:1000:12: error: 'sirfsoc_dma_runtime_resume' defined but not used [-Werror=unused-function] This removes the existing #ifdef and instead uses __maybe_unused annotations for all four functions to let the compiler know it can silently drop the function definition. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/dma/sirf-dma.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)