Linux Kernel Mentee: Remove Legacy Power Management.
The purpose of this patch series is to remove legacy power management callbacks
from amd ethernet drivers.
The callbacks performing suspend() and resume() operations are still calling
pci_save_state(), pci_set_power_state(), etc. and handling the power management
themselves, which is not recommended.
The conversion requires the removal of the those function calls and change the
callback definition accordingly and make use of dev_pm_ops structure.
All patches are compile-tested only.
Vaibhav Gupta (3):
pcnet32: Convert to generic power management
amd8111e: Convert to generic power mangement
amd-xgbe: Convert to generic power management
drivers/net/ethernet/amd/amd8111e.c | 30 +++++++++---------------
drivers/net/ethernet/amd/pcnet32.c | 22 ++++++++---------
drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 17 +++++++-------
3 files changed, 31 insertions(+), 38 deletions(-)
--
2.26.2
compile-tested only
Drivers should not save device registers and/or change the power state of
the device. As per the generic PM approach, these are handled by PCI core.
The driver should support dev_pm_ops callbacks and bind them to pci_driver.
Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/net/ethernet/amd/amd8111e.c | 30 +++++++++++------------------
1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/amd/amd8111e.c b/drivers/net/ethernet/amd/amd8111e.c
index 7a1286f8e983..c6591b33abcc 100644
--- a/drivers/net/ethernet/amd/amd8111e.c
+++ b/drivers/net/ethernet/amd/amd8111e.c
@@ -1580,9 +1580,10 @@ static void amd8111e_tx_timeout(struct net_device *dev, unsigned int txqueue)
if(!err)
netif_wake_queue(dev);
}
-static int amd8111e_suspend(struct pci_dev *pci_dev, pm_message_t state)
+
+static int amd8111e_suspend(struct device *dev_d)
{
- struct net_device *dev = pci_get_drvdata(pci_dev);
+ struct net_device *dev = dev_get_drvdata(dev_d);
struct amd8111e_priv *lp = netdev_priv(dev);
if (!netif_running(dev))
@@ -1609,34 +1610,24 @@ static int amd8111e_suspend(struct pci_dev *pci_dev, pm_message_t state)
if(lp->options & OPTION_WAKE_PHY_ENABLE)
amd8111e_enable_link_change(lp);
- pci_enable_wake(pci_dev, PCI_D3hot, 1);
- pci_enable_wake(pci_dev, PCI_D3cold, 1);
+ device_set_wakeup_enable(dev_d, 1);
}
else{
- pci_enable_wake(pci_dev, PCI_D3hot, 0);
- pci_enable_wake(pci_dev, PCI_D3cold, 0);
+ device_set_wakeup_enable(dev_d, 0);
}
- pci_save_state(pci_dev);
- pci_set_power_state(pci_dev, PCI_D3hot);
-
return 0;
}
-static int amd8111e_resume(struct pci_dev *pci_dev)
+
+static int amd8111e_resume(struct device *dev_d)
{
- struct net_device *dev = pci_get_drvdata(pci_dev);
+ struct net_device *dev = dev_get_drvdata(dev_d);
struct amd8111e_priv *lp = netdev_priv(dev);
if (!netif_running(dev))
return 0;
- pci_set_power_state(pci_dev, PCI_D0);
- pci_restore_state(pci_dev);
-
- pci_enable_wake(pci_dev, PCI_D3hot, 0);
- pci_enable_wake(pci_dev, PCI_D3cold, 0); /* D3 cold */
-
netif_device_attach(dev);
spin_lock_irq(&lp->lock);
@@ -1918,13 +1909,14 @@ static const struct pci_device_id amd8111e_pci_tbl[] = {
};
MODULE_DEVICE_TABLE(pci, amd8111e_pci_tbl);
+static SIMPLE_DEV_PM_OPS(amd8111e_pm_ops, amd8111e_suspend, amd8111e_resume);
+
static struct pci_driver amd8111e_driver = {
.name = MODULE_NAME,
.id_table = amd8111e_pci_tbl,
.probe = amd8111e_probe_one,
.remove = amd8111e_remove_one,
- .suspend = amd8111e_suspend,
- .resume = amd8111e_resume
+ .driver.pm = &amd8111e_pm_ops
};
module_pci_driver(amd8111e_driver);
--
2.26.2
compile-tested only
Use dev_pm_ops structure to call generic suspend() and resume() callbacks.
Drivers should avoid saving device register and/or change power states
using PCI helper functions. With generic approach, all these are handled by
PCI core.
Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
index 7b86240ecd5f..014cee31a1d4 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
@@ -421,9 +421,9 @@ static void xgbe_pci_remove(struct pci_dev *pdev)
xgbe_free_pdata(pdata);
}
-#ifdef CONFIG_PM
-static int xgbe_pci_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused xgbe_pci_suspend(struct device *dev)
{
+ struct pci_dev *pdev = to_pci_dev(dev);
struct xgbe_prv_data *pdata = pci_get_drvdata(pdev);
struct net_device *netdev = pdata->netdev;
int ret = 0;
@@ -438,8 +438,9 @@ static int xgbe_pci_suspend(struct pci_dev *pdev, pm_message_t state)
return ret;
}
-static int xgbe_pci_resume(struct pci_dev *pdev)
+static int __maybe_unused xgbe_pci_resume(struct device *dev)
{
+ struct pci_dev *pdev = to_pci_dev(dev);
struct xgbe_prv_data *pdata = pci_get_drvdata(pdev);
struct net_device *netdev = pdata->netdev;
int ret = 0;
@@ -460,7 +461,6 @@ static int xgbe_pci_resume(struct pci_dev *pdev)
return ret;
}
-#endif /* CONFIG_PM */
static const struct xgbe_version_data xgbe_v2a = {
.init_function_ptrs_phy_impl = xgbe_init_function_ptrs_phy_v2,
@@ -502,15 +502,16 @@ static const struct pci_device_id xgbe_pci_table[] = {
};
MODULE_DEVICE_TABLE(pci, xgbe_pci_table);
+static SIMPLE_DEV_PM_OPS(xgbe_pci_pm_ops, xgbe_pci_suspend, xgbe_pci_resume);
+
static struct pci_driver xgbe_driver = {
.name = XGBE_DRV_NAME,
.id_table = xgbe_pci_table,
.probe = xgbe_pci_probe,
.remove = xgbe_pci_remove,
-#ifdef CONFIG_PM
- .suspend = xgbe_pci_suspend,
- .resume = xgbe_pci_resume,
-#endif
+ .driver = {
+ .pm = &xgbe_pci_pm_ops,
+ }
};
int xgbe_pci_init(void)
--
2.26.2
compile-tested only
Remove legacy PM callbacks and use generic operations. With legacy code,
drivers were responsible for handling PCI PM operations like
"pci_save_state()". In generic code, all these handled by PCI core.
The generic "suspend()" and "resume()" are called at the same point the
legacy ones were called. Thus, it does not affect the normal functioning of
the driver.
Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/net/ethernet/amd/pcnet32.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/amd/pcnet32.c b/drivers/net/ethernet/amd/pcnet32.c
index 07e8211eea51..d32f54d760e7 100644
--- a/drivers/net/ethernet/amd/pcnet32.c
+++ b/drivers/net/ethernet/amd/pcnet32.c
@@ -2913,30 +2913,27 @@ static void pcnet32_watchdog(struct timer_list *t)
mod_timer(&lp->watchdog_timer, round_jiffies(PCNET32_WATCHDOG_TIMEOUT));
}
-static int pcnet32_pm_suspend(struct pci_dev *pdev, pm_message_t state)
+static int pcnet32_pm_suspend(struct device *device_d)
{
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct net_device *dev = dev_get_drvdata(device_d);
if (netif_running(dev)) {
netif_device_detach(dev);
pcnet32_close(dev);
}
- pci_save_state(pdev);
- pci_set_power_state(pdev, pci_choose_state(pdev, state));
+
return 0;
}
-static int pcnet32_pm_resume(struct pci_dev *pdev)
+static int pcnet32_pm_resume(struct device *device_d)
{
- struct net_device *dev = pci_get_drvdata(pdev);
-
- pci_set_power_state(pdev, PCI_D0);
- pci_restore_state(pdev);
+ struct net_device *dev = dev_get_drvdata(device_d);
if (netif_running(dev)) {
pcnet32_open(dev);
netif_device_attach(dev);
}
+
return 0;
}
@@ -2957,13 +2954,16 @@ static void pcnet32_remove_one(struct pci_dev *pdev)
}
}
+static SIMPLE_DEV_PM_OPS(pcnet32_pm_ops, pcnet32_pm_suspend, pcnet32_pm_resume);
+
static struct pci_driver pcnet32_driver = {
.name = DRV_NAME,
.probe = pcnet32_probe_pci,
.remove = pcnet32_remove_one,
.id_table = pcnet32_pci_tbl,
- .suspend = pcnet32_pm_suspend,
- .resume = pcnet32_pm_resume,
+ .driver = {
+ .pm = &pcnet32_pm_ops,
+ },
};
/* An additional parameter that may be passed in... */
--
2.26.2