Linux Kernel Mentee: Remove Legacy Power Management.
The purpose of this patch series is to remove legacy power management callbacks
from atheros 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 (2):
atl1e: use generic power management
atl2: use generic power management
.../net/ethernet/atheros/atl1e/atl1e_main.c | 53 ++++-----------
drivers/net/ethernet/atheros/atlx/atl2.c | 64 ++++++-------------
2 files changed, 31 insertions(+), 86 deletions(-)
--
2.27.0
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 hre andled 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.
__maybe_unused attribute is used with .resume() but not with .suspend(), as
.suspend() is calleb by .shutdown().
Compile-tested only.
Signed-off-by: Vaibhav Gupta <[email protected]>
---
.../net/ethernet/atheros/atl1e/atl1e_main.c | 53 +++++--------------
1 file changed, 13 insertions(+), 40 deletions(-)
diff --git a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
index 223ef846123e..9b03299ba665 100644
--- a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
+++ b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
@@ -2048,9 +2048,9 @@ static int atl1e_close(struct net_device *netdev)
return 0;
}
-static int atl1e_suspend(struct pci_dev *pdev, pm_message_t state)
+static int atl1e_suspend(struct device *dev_d)
{
- struct net_device *netdev = pci_get_drvdata(pdev);
+ struct net_device *netdev = dev_get_drvdata(dev_d);
struct atl1e_adapter *adapter = netdev_priv(netdev);
struct atl1e_hw *hw = &adapter->hw;
u32 ctrl = 0;
@@ -2061,9 +2061,6 @@ static int atl1e_suspend(struct pci_dev *pdev, pm_message_t state)
u16 mii_intr_status_data = 0;
u32 wufc = adapter->wol;
u32 i;
-#ifdef CONFIG_PM
- int retval = 0;
-#endif
if (netif_running(netdev)) {
WARN_ON(test_bit(__AT_RESETTING, &adapter->flags));
@@ -2071,12 +2068,6 @@ static int atl1e_suspend(struct pci_dev *pdev, pm_message_t state)
}
netif_device_detach(netdev);
-#ifdef CONFIG_PM
- retval = pci_save_state(pdev);
- if (retval)
- return retval;
-#endif
-
if (wufc) {
/* get link status */
atl1e_read_phy_reg(hw, MII_BMSR, &mii_bmsr_data);
@@ -2146,7 +2137,7 @@ static int atl1e_suspend(struct pci_dev *pdev, pm_message_t state)
ctrl = AT_READ_REG(hw, REG_PCIE_PHYMISC);
ctrl |= PCIE_PHYMISC_FORCE_RCV_DET;
AT_WRITE_REG(hw, REG_PCIE_PHYMISC, ctrl);
- pci_enable_wake(pdev, pci_choose_state(pdev, state), 1);
+ device_wakeup_enable(dev_d);
goto suspend_exit;
}
wol_dis:
@@ -2162,43 +2153,27 @@ static int atl1e_suspend(struct pci_dev *pdev, pm_message_t state)
atl1e_force_ps(hw);
hw->phy_configured = false; /* re-init PHY when resume */
- pci_enable_wake(pdev, pci_choose_state(pdev, state), 0);
+ device_wakeup_disable(dev_d);
suspend_exit:
if (netif_running(netdev))
atl1e_free_irq(adapter);
- pci_disable_device(pdev);
-
- pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
return 0;
}
-#ifdef CONFIG_PM
-static int atl1e_resume(struct pci_dev *pdev)
+static int __maybe_unused atl1e_resume(struct device *dev_d)
{
- struct net_device *netdev = pci_get_drvdata(pdev);
+ struct net_device *netdev = dev_get_drvdata(dev_d);
struct atl1e_adapter *adapter = netdev_priv(netdev);
u32 err;
- pci_set_power_state(pdev, PCI_D0);
- pci_restore_state(pdev);
-
- err = pci_enable_device(pdev);
- if (err) {
- netdev_err(adapter->netdev,
- "Cannot enable PCI device from suspend\n");
- return err;
- }
-
- pci_set_master(pdev);
+ pci_set_master(to_pci_dev(dev_d));
AT_READ_REG(&adapter->hw, REG_WOL_CTRL); /* clear WOL status */
- pci_enable_wake(pdev, PCI_D3hot, 0);
- pci_enable_wake(pdev, PCI_D3cold, 0);
+ device_wakeup_disable(dev_d);
AT_WRITE_REG(&adapter->hw, REG_WOL_CTRL, 0);
@@ -2217,11 +2192,10 @@ static int atl1e_resume(struct pci_dev *pdev)
return 0;
}
-#endif
static void atl1e_shutdown(struct pci_dev *pdev)
{
- atl1e_suspend(pdev, PMSG_SUSPEND);
+ atl1e_suspend(&pdev->dev);
}
static const struct net_device_ops atl1e_netdev_ops = {
@@ -2533,16 +2507,15 @@ static const struct pci_error_handlers atl1e_err_handler = {
.resume = atl1e_io_resume,
};
+static SIMPLE_DEV_PM_OPS(atl1e_pm_ops, atl1e_suspend, atl1e_resume);
+
static struct pci_driver atl1e_driver = {
.name = atl1e_driver_name,
.id_table = atl1e_pci_tbl,
.probe = atl1e_probe,
.remove = atl1e_remove,
- /* Power Management Hooks */
-#ifdef CONFIG_PM
- .suspend = atl1e_suspend,
- .resume = atl1e_resume,
-#endif
+ /* Power Management Hook */
+ .driver.pm = &atl1e_pm_ops,
.shutdown = atl1e_shutdown,
.err_handler = &atl1e_err_handler
};
--
2.27.0
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 hre andled 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.
__maybe_unused attribute is used with .resume() but not with .suspend(), as
.suspend() is calleb by .shutdown().
Compile-tested only.
Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/net/ethernet/atheros/atlx/atl2.c | 64 +++++++-----------------
1 file changed, 18 insertions(+), 46 deletions(-)
diff --git a/drivers/net/ethernet/atheros/atlx/atl2.c b/drivers/net/ethernet/atheros/atlx/atl2.c
index c915852b8892..3e01e9fdb7fd 100644
--- a/drivers/net/ethernet/atheros/atlx/atl2.c
+++ b/drivers/net/ethernet/atheros/atlx/atl2.c
@@ -1484,19 +1484,15 @@ static void atl2_remove(struct pci_dev *pdev)
pci_disable_device(pdev);
}
-static int atl2_suspend(struct pci_dev *pdev, pm_message_t state)
+static int atl2_suspend(struct device *dev_d)
{
- struct net_device *netdev = pci_get_drvdata(pdev);
+ struct net_device *netdev = dev_get_drvdata(dev_d);
struct atl2_adapter *adapter = netdev_priv(netdev);
struct atl2_hw *hw = &adapter->hw;
u16 speed, duplex;
u32 ctrl = 0;
u32 wufc = adapter->wol;
-#ifdef CONFIG_PM
- int retval = 0;
-#endif
-
netif_device_detach(netdev);
if (netif_running(netdev)) {
@@ -1504,12 +1500,6 @@ static int atl2_suspend(struct pci_dev *pdev, pm_message_t state)
atl2_down(adapter);
}
-#ifdef CONFIG_PM
- retval = pci_save_state(pdev);
- if (retval)
- return retval;
-#endif
-
atl2_read_phy_reg(hw, MII_BMSR, (u16 *)&ctrl);
atl2_read_phy_reg(hw, MII_BMSR, (u16 *)&ctrl);
if (ctrl & BMSR_LSTATUS)
@@ -1560,7 +1550,7 @@ static int atl2_suspend(struct pci_dev *pdev, pm_message_t state)
ctrl |= PCIE_DLL_TX_CTRL1_SEL_NOR_CLK;
ATL2_WRITE_REG(hw, REG_PCIE_DLL_TX_CTRL1, ctrl);
- pci_enable_wake(pdev, pci_choose_state(pdev, state), 1);
+ device_wakeup_enable(dev_d);
goto suspend_exit;
}
@@ -1580,7 +1570,7 @@ static int atl2_suspend(struct pci_dev *pdev, pm_message_t state)
hw->phy_configured = false; /* re-init PHY when resume */
- pci_enable_wake(pdev, pci_choose_state(pdev, state), 1);
+ device_wakeup_enable(dev_d);
goto suspend_exit;
}
@@ -1600,42 +1590,26 @@ static int atl2_suspend(struct pci_dev *pdev, pm_message_t state)
atl2_force_ps(hw);
hw->phy_configured = false; /* re-init PHY when resume */
- pci_enable_wake(pdev, pci_choose_state(pdev, state), 0);
+ device_wakeup_disable(dev_d);
suspend_exit:
if (netif_running(netdev))
atl2_free_irq(adapter);
- pci_disable_device(pdev);
-
- pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
return 0;
}
-#ifdef CONFIG_PM
-static int atl2_resume(struct pci_dev *pdev)
+static int __maybe_unused atl2_resume(struct device *dev_d)
{
- struct net_device *netdev = pci_get_drvdata(pdev);
+ struct net_device *netdev = dev_get_drvdata(dev_d);
struct atl2_adapter *adapter = netdev_priv(netdev);
u32 err;
- pci_set_power_state(pdev, PCI_D0);
- pci_restore_state(pdev);
-
- err = pci_enable_device(pdev);
- if (err) {
- printk(KERN_ERR
- "atl2: Cannot enable PCI device from suspend\n");
- return err;
- }
-
- pci_set_master(pdev);
+ pci_set_master(to_pci_dev(dev_d));
ATL2_READ_REG(&adapter->hw, REG_WOL_CTRL); /* clear WOL status */
- pci_enable_wake(pdev, PCI_D3hot, 0);
- pci_enable_wake(pdev, PCI_D3cold, 0);
+ device_wakeup_disable(dev_d);
ATL2_WRITE_REG(&adapter->hw, REG_WOL_CTRL, 0);
@@ -1654,24 +1628,22 @@ static int atl2_resume(struct pci_dev *pdev)
return 0;
}
-#endif
static void atl2_shutdown(struct pci_dev *pdev)
{
- atl2_suspend(pdev, PMSG_SUSPEND);
+ atl2_suspend(&pdev->dev);
}
+static SIMPLE_DEV_PM_OPS(atl2_pm_ops, atl2_suspend, atl2_resume);
+
static struct pci_driver atl2_driver = {
- .name = atl2_driver_name,
- .id_table = atl2_pci_tbl,
- .probe = atl2_probe,
- .remove = atl2_remove,
+ .name = atl2_driver_name,
+ .id_table = atl2_pci_tbl,
+ .probe = atl2_probe,
+ .remove = atl2_remove,
/* Power Management Hooks */
- .suspend = atl2_suspend,
-#ifdef CONFIG_PM
- .resume = atl2_resume,
-#endif
- .shutdown = atl2_shutdown,
+ .driver.pm = &atl2_pm_ops,
+ .shutdown = atl2_shutdown,
};
/**
--
2.27.0