diff mbox

[06/14] dma: sirf: use __maybe_unused to hide pm functions

Message ID 1456934350-1389172-7-git-send-email-arnd@arndb.de (mailing list archive)
State Accepted
Headers show

Commit Message

Arnd Bergmann March 2, 2016, 3:58 p.m. UTC
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(-)

Comments

Vinod Koul March 3, 2016, 3:47 a.m. UTC | #1
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?
Arnd Bergmann March 3, 2016, 12:33 p.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vinod Koul March 4, 2016, 3:02 p.m. UTC | #3
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 mbox

Patch

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)