The amd-xgbe ethernet driver hides its suspend/resume functions
in #ifdef CONFIG_PM, but uses SIMPLE_DEV_PM_OPS() to make the
reference conditional on CONFIG_PM_SLEEP, which results in a
warning when PM_SLEEP is not set but PM is:
drivers/net/ethernet/amd/xgbe/xgbe-platform.c:553:12: error: 'xgbe_platform_resume' defined but not used [-Werror=unused-function]
drivers/net/ethernet/amd/xgbe/xgbe-platform.c:533:12: error: 'xgbe_platform_suspend' defined but not used [-Werror=unused-function]
This removes the incorrect #ifdef and instead uses a __maybe_unused
annotation to let the compiler know it can silently drop
the function definition.
Fixes: bd8255d8ba35 ("amd-xgbe: Prepare for supporting PCI devices")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I originally submitted this when in March 2016, but the patch has not
yet made it upstream, and the file contents have moved around so
the old patch no longer applied so I'm resending the rebased version
now.
---
drivers/net/ethernet/amd/xgbe/xgbe-platform.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-platform.c b/drivers/net/ethernet/amd/xgbe/xgbe-platform.c
index 0edbcd523f8f..02daca817bb7 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-platform.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-platform.c
@@ -529,8 +529,7 @@ static int xgbe_platform_remove(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_PM
-static int xgbe_platform_suspend(struct device *dev)
+static int __maybe_unused xgbe_platform_suspend(struct device *dev)
{
struct xgbe_prv_data *pdata = dev_get_drvdata(dev);
struct net_device *netdev = pdata->netdev;@@ -550,7 +549,7 @@ static int xgbe_platform_suspend(struct device *dev)
return ret;
}
-static int xgbe_platform_resume(struct device *dev)
+static int __maybe_unused xgbe_platform_resume(struct device *dev)
{
struct xgbe_prv_data *pdata = dev_get_drvdata(dev);
struct net_device *netdev = pdata->netdev;@@ -574,7 +573,6 @@ static int xgbe_platform_resume(struct device *dev)
return ret;
}
-#endif /* CONFIG_PM */
static const struct xgbe_version_data xgbe_v1 = {
.init_function_ptrs_phy_impl = xgbe_init_function_ptrs_phy_v1,--
2.9.0
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 8 Nov 2016 14:37:32 +0100
The amd-xgbe ethernet driver hides its suspend/resume functions
in #ifdef CONFIG_PM, but uses SIMPLE_DEV_PM_OPS() to make the
reference conditional on CONFIG_PM_SLEEP, which results in a
warning when PM_SLEEP is not set but PM is:
drivers/net/ethernet/amd/xgbe/xgbe-platform.c:553:12: error: 'xgbe_platform_resume' defined but not used [-Werror=unused-function]
drivers/net/ethernet/amd/xgbe/xgbe-platform.c:533:12: error: 'xgbe_platform_suspend' defined but not used [-Werror=unused-function]
This removes the incorrect #ifdef and instead uses a __maybe_unused
annotation to let the compiler know it can silently drop
the function definition.
Fixes: bd8255d8ba35 ("amd-xgbe: Prepare for supporting PCI devices")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I originally submitted this when in March 2016, but the patch has not
yet made it upstream, and the file contents have moved around so
the old patch no longer applied so I'm resending the rebased version
now.
By and large, drivers handle this by using a CONFIG_PM_SLEEP ifdef.
Unless you can make an extremely convincing argument why not to do
so here, I'd like you to handle it that way instead.
Thanks.
On Wednesday, November 9, 2016 8:32:51 PM CET David Miller wrote:
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 8 Nov 2016 14:37:32 +0100
quoted
The amd-xgbe ethernet driver hides its suspend/resume functions
in #ifdef CONFIG_PM, but uses SIMPLE_DEV_PM_OPS() to make the
reference conditional on CONFIG_PM_SLEEP, which results in a
warning when PM_SLEEP is not set but PM is:
drivers/net/ethernet/amd/xgbe/xgbe-platform.c:553:12: error: 'xgbe_platform_resume' defined but not used [-Werror=unused-function]
drivers/net/ethernet/amd/xgbe/xgbe-platform.c:533:12: error: 'xgbe_platform_suspend' defined but not used [-Werror=unused-function]
This removes the incorrect #ifdef and instead uses a __maybe_unused
annotation to let the compiler know it can silently drop
the function definition.
Fixes: bd8255d8ba35 ("amd-xgbe: Prepare for supporting PCI devices")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
I originally submitted this when in March 2016, but the patch has not
yet made it upstream, and the file contents have moved around so
the old patch no longer applied so I'm resending the rebased version
now.
By and large, drivers handle this by using a CONFIG_PM_SLEEP ifdef.
Unless you can make an extremely convincing argument why not to do
so here, I'd like you to handle it that way instead.
[adding linux-pm to Cc]
The main reason is that everyone gets the #ifdef wrong, I run into
half a dozen new build regressions with linux-next every week on
average, the typical problems being:
- testing CONFIG_PM_SLEEP instead of CONFIG_PM, leading to an unused
function warning
- testing CONFIG_PM instead of CONFIG_PM_SLEEP, leading to a build
failure
- calling a function outside of the #ifdef only from inside an
otherwise correct #ifdef, again leading to an unused function
warning
- causing a warning inside of the #ifdef but only testing if that
is disabled, leading to a problem if the macro is set (this is
rare these days for CONFIG_PM as that is normally enabled)
Using __maybe_unused avoids all of the above. My plan for the
long run however is to change the macro to something like
#define SET_SYSTEM_SLEEP_PM_OPS(suspend_fn, resume_fn) \
.suspend = IS_ENABLED(CONFIG_PM_SLEEP) ? suspend_fn : NULL, \
.resume = IS_ENABLED(CONFIG_PM_SLEEP) ? resume_fn : NULL, \
.freeze = IS_ENABLED(CONFIG_PM_SLEEP) ? suspend_fn : NULL, \
.thaw = IS_ENABLED(CONFIG_PM_SLEEP) ? resume_fn : NULL, \
.poweroff = IS_ENABLED(CONFIG_PM_SLEEP) ? suspend_fn : NULL, \
.restore = IS_ENABLED(CONFIG_PM_SLEEP) ? resume_fn : NULL,
#define SET_RUNTIME_PM_OPS(suspend_fn, resume_fn, idle_fn) \
.runtime_suspend = IS_ENABLED(CONFIG_PM) ? suspend_fn : NULL, \
.runtime_resume = IS_ENABLED(CONFIG_PM) ? resume_fn : NULL, \
.runtime_idle = IS_ENABLED(CONFIG_PM) ? idle_fn : NULL,
I just haven't found the energy to start that discussion. With a definition
like this, we would need neither #ifdef nor __maybe_unused. Unfortunately
we can't just replace the existing macro while keeping the same name
because that would break every single user that has the #ifdef.
There was some discussion about that a while ago but no patch was merged
for it. I think in order to pull this off, we'd have to introduced
replacements for the existing six macros and change over the ~1000
existing users before removing the existing definitions:
202 SET_SYSTEM_SLEEP_PM_OPS
14 SET_LATE_SYSTEM_SLEEP_PM_OPS
11 SET_NOIRQ_SYSTEM_SLEEP_PM_OPS
218 SET_RUNTIME_PM_OPS
551 SIMPLE_DEV_PM_OPS
12 UNIVERSAL_DEV_PM_OPS
This would of course be a nontrivial amount of work, but it could
be mostly automated using coccinelle. In files per subsystem,
this would be
7 drivers/acpi
14 drivers/ata
17 drivers/char
6 drivers/crypto
26 drivers/dma
7 drivers/extcon
7 drivers/gpio
41 drivers/gpu
10 drivers/hwmon
7 drivers/hwtracing
29 drivers/i2c
90 drivers/iio
37 drivers/media
28 drivers/mfd
15 drivers/misc
52 drivers/mmc
11 drivers/mtd
67 drivers/net
10 drivers/pinctrl
19 drivers/platform
13 drivers/power
7 drivers/pwm
44 drivers/rtc
46 drivers/spi
15 drivers/staging
11 drivers/thermal
22 drivers/tty
37 drivers/usb
32 drivers/video
15 drivers/watchdog
38 sound/pci
52 sound/soc
Arnd