[PATCH] dwc3: make PM functions as __maybe_unused

[PATCH] dwc3: make PM functions as __maybe_unused

[Arnd Bergmann]

A change to the suspend/resume handling in dwc3-pci introduced a
harmless warning:

drivers/usb/dwc3/dwc3-pci.c:169:12: error: ‘dwc3_pci_dsm’ defined but not used [-Werror=unused-function]

Replacing the #ifdef around the PM functions with __maybe_unused
annotations is the easiest way to make sure this doesn't happen
again. A similar problem happened two months earlier and we
ended up updating the #ifdef, but as it has come back now,
I'd suggest going back to my earlier approach.

Fixes: 9cecca75b5a0 ("usb: dwc3: pci: call _DSM for suspend/resume")
Link: https://patchwork.kernel.org/patch/9318887/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/usb/dwc3/dwc3-pci.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 8c39ec6522fd..771b620b4878 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -293,8 +293,7 @@ static const struct pci_device_id dwc3_pci_id_table[] = {
 };
 MODULE_DEVICE_TABLE(pci, dwc3_pci_id_table);
 
-#ifdef CONFIG_PM
-static int dwc3_pci_runtime_suspend(struct device *dev)
+static int __maybe_unused dwc3_pci_runtime_suspend(struct device *dev)
 {
 	struct dwc3_pci		*dwc = dev_get_drvdata(dev);
 
@@ -304,7 +303,7 @@ static int dwc3_pci_runtime_suspend(struct device *dev)
 	return -EBUSY;
 }
 
-static int dwc3_pci_runtime_resume(struct device *dev)
+static int __maybe_unused dwc3_pci_runtime_resume(struct device *dev)
 {
 	struct dwc3_pci		*dwc = dev_get_drvdata(dev);
 	struct platform_device	*dwc3 = dwc->dwc3;
@@ -316,23 +315,20 @@ static int dwc3_pci_runtime_resume(struct device *dev)
 
 	return pm_runtime_get(&dwc3->dev);
 }
-#endif /* CONFIG_PM */
 
-#ifdef CONFIG_PM_SLEEP
-static int dwc3_pci_suspend(struct device *dev)
+static int __maybe_unused dwc3_pci_suspend(struct device *dev)
 {
 	struct dwc3_pci		*dwc = dev_get_drvdata(dev);
 
 	return dwc3_pci_dsm(dwc, PCI_INTEL_BXT_STATE_D3);
 }
 
-static int dwc3_pci_resume(struct device *dev)
+static int __maybe_unused dwc3_pci_resume(struct device *dev)
 {
 	struct dwc3_pci		*dwc = dev_get_drvdata(dev);
 
 	return dwc3_pci_dsm(dwc, PCI_INTEL_BXT_STATE_D0);
 }
-#endif /* CONFIG_PM_SLEEP */
 
 static struct dev_pm_ops dwc3_pci_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(dwc3_pci_suspend, dwc3_pci_resume)
-- 
2.9.0

Re: [PATCH] dwc3: make PM functions as __maybe_unused

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 985 bytes --]


Hi,

Arnd Bergmann <arnd@arndb.de> writes:
> A change to the suspend/resume handling in dwc3-pci introduced a
> harmless warning:
>
> drivers/usb/dwc3/dwc3-pci.c:169:12: error: ‘dwc3_pci_dsm’ defined but not used [-Werror=unused-function]
>
> Replacing the #ifdef around the PM functions with __maybe_unused
> annotations is the easiest way to make sure this doesn't happen
> again. A similar problem happened two months earlier and we
> ended up updating the #ifdef, but as it has come back now,
> I'd suggest going back to my earlier approach.
>
> Fixes: 9cecca75b5a0 ("usb: dwc3: pci: call _DSM for suspend/resume")
> Link: https://patchwork.kernel.org/patch/9318887/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

I'll just move the ifdef around. We really need a real fix for this. Why
couldn't we just always add PM callbacks and assume they won't be used
if !PM && !PM_SLEEP?

Adding __maybe_unused everywhere is rather unelegant :-(

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] dwc3: make PM functions as __maybe_unused

[Arnd Bergmann]

On Wednesday, November 16, 2016 1:13:48 PM CET Felipe Balbi wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
> > A change to the suspend/resume handling in dwc3-pci introduced a
> > harmless warning:
> >
> > drivers/usb/dwc3/dwc3-pci.c:169:12: error: ‘dwc3_pci_dsm’ defined but not used [-Werror=unused-function]
> >
> > Replacing the #ifdef around the PM functions with __maybe_unused
> > annotations is the easiest way to make sure this doesn't happen
> > again. A similar problem happened two months earlier and we
> > ended up updating the #ifdef, but as it has come back now,
> > I'd suggest going back to my earlier approach.
> >
> > Fixes: 9cecca75b5a0 ("usb: dwc3: pci: call _DSM for suspend/resume")
> > Link: https://patchwork.kernel.org/patch/9318887/
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> I'll just move the ifdef around. We really need a real fix for this. Why
> couldn't we just always add PM callbacks and assume they won't be used
> if !PM && !PM_SLEEP?

I fully agree. This is now the most common warning that gets added
to linux-next, and there are sometimes several new ones on one day, so
I'm playing whack-a-mole here.

I have a rough plan for a proper solution already, but it's going to
be hard to do since there are so many drivers using the current
method. See https://www.mail-archive.com/netdev@vger.kernel.org/msg136759.html
for what I suggested last week.

> Adding __maybe_unused everywhere is rather unelegant 

I agree, we shouldn't need the #ifdef or the __maybe_unused. I always
use the latter since it's easier to get right. In this particular case,
if you had applied the original patch, it the warning would not have
come back now.

	Arnd