* [net-next PATCH] amd-xgbe: use __maybe_unused to hide pm functions
@ 2016-11-08 13:37 Arnd Bergmann
2016-11-10 1:32 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Arnd Bergmann @ 2016-11-08 13:37 UTC (permalink / raw)
To: Tom Lendacky; +Cc: Arnd Bergmann, David S. Miller, netdev, linux-kernel
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [net-next PATCH] amd-xgbe: use __maybe_unused to hide pm functions
2016-11-08 13:37 [net-next PATCH] amd-xgbe: use __maybe_unused to hide pm functions Arnd Bergmann
@ 2016-11-10 1:32 ` David Miller
2016-11-10 10:47 ` Arnd Bergmann
0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2016-11-10 1:32 UTC (permalink / raw)
To: arnd; +Cc: thomas.lendacky, netdev, linux-kernel
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [net-next PATCH] amd-xgbe: use __maybe_unused to hide pm functions
2016-11-10 1:32 ` David Miller
@ 2016-11-10 10:47 ` Arnd Bergmann
0 siblings, 0 replies; 3+ messages in thread
From: Arnd Bergmann @ 2016-11-10 10:47 UTC (permalink / raw)
To: David Miller
Cc: thomas.lendacky, netdev, linux-kernel, Linux PM,
Rafael J. Wysocki, Kirtika Ruchandani
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
>
> > 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-11-10 10:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-08 13:37 [net-next PATCH] amd-xgbe: use __maybe_unused to hide pm functions Arnd Bergmann
2016-11-10 1:32 ` David Miller
2016-11-10 10:47 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).