[PATCH v2] net: ethernet: bgmac: use pm_ptr() to clean up PM code

Pengpeng Hou posted 1 patch 4 weeks, 1 day ago
drivers/net/ethernet/broadcom/bgmac-platform.c | 21 +++++----------------
1 file changed, 5 insertions(+), 16 deletions(-)
[PATCH v2] net: ethernet: bgmac: use pm_ptr() to clean up PM code
Posted by Pengpeng Hou 4 weeks, 1 day ago
Hi Andrew,

Thanks for the tip! Using pm_ptr() is definitely a better way to handle
this. It cleans up the code nicely and keeps the build tests happy even
when PM is disabled.

In the current code, bgmac_enet_suspend() and bgmac_enet_resume() are
always there in the core driver, but their only users in
bgmac-platform.c were wrapped in #ifdef CONFIG_PM.

As you suggested, I've swapped the #ifdef guards for pm_ptr() and
marked the platform wrappers with __maybe_unused. This way, the compiler
always sees the code, but the linker will still toss it out if power
management isn't being used.

Signed-off-by: Pengpeng Hou <pengpeng.hou@isrc.iscas.ac.cn>
---
Changes in v2:
- Switched from #ifdef guards to pm_ptr() as Andrew suggested.
- Cleaned up the BGMAC_PM_OPS macro and used __maybe_unused for the
  callback functions. It looks much better now!

 drivers/net/ethernet/broadcom/bgmac-platform.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c
index 86770f1..f9c32e1 100644
--- a/drivers/net/ethernet/broadcom/bgmac-platform.c
+++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
@@ -257,33 +257,22 @@ static void bgmac_remove(struct platform_device *pdev)
 	bgmac_enet_remove(bgmac);
 }
 
-#ifdef CONFIG_PM
-static int bgmac_suspend(struct device *dev)
+static int __maybe_unused bgmac_suspend(struct device *dev)
 {
 	struct bgmac *bgmac = dev_get_drvdata(dev);
 
 	return bgmac_enet_suspend(bgmac);
 }
 
-static int bgmac_resume(struct device *dev)
+static int __maybe_unused bgmac_resume(struct device *dev)
 {
 	struct bgmac *bgmac = dev_get_drvdata(dev);
 
 	return bgmac_enet_resume(bgmac);
 }
 
 static const struct dev_pm_ops bgmac_pm_ops = {
-	.suspend = bgmac_suspend,
-	.resume = bgmac_resume
+	SET_SYSTEM_SLEEP_PM_OPS(bgmac_suspend, bgmac_resume)
 };
-
-#define BGMAC_PM_OPS (&bgmac_pm_ops)
-#else
-#define BGMAC_PM_OPS NULL
-#endif /* CONFIG_PM */
 
 static const struct of_device_id bgmac_of_enet_match[] = {
 	{.compatible = "brcm,amac",},
@@ -299,7 +288,7 @@ static struct platform_driver bgmac_enet_driver = {
 	.driver = {
 		.name  = "bgmac-enet",
 		.of_match_table = bgmac_of_enet_match,
-		.pm = BGMAC_PM_OPS
+		.pm = pm_ptr(&bgmac_pm_ops),
 	},
 	.probe = bgmac_probe,
 	.remove = bgmac_remove,
--
Re: [PATCH v2] net: ethernet: bgmac: use pm_ptr() to clean up PM code
Posted by Andrew Lunn 4 weeks, 1 day ago
> As you suggested, I've swapped the #ifdef guards for pm_ptr() and
> marked the platform wrappers with __maybe_unused. This way, the compiler
> always sees the code, but the linker will still toss it out if power
> management isn't being used.

Think about that some more. If the compiler always sees the code, and
then throws it out at the optimizer stage, why do you need
__maybe_unused?

Do you try building this without the __maybe_unused.? Did you look at
other examples of pm_ptr()? Do they also have __maybe_used?

      Andrew