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 (4):
qlge/qlge_main.c: use genric power management
staging/rtl8192e: use generic power management
rts5208/rtsx.c: use generic power management
vt6655/device_main.c: use generic power management
drivers/staging/qlge/qlge_main.c | 36 +++++---------------
drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 5 +--
drivers/staging/rtl8192e/rtl8192e/rtl_pm.c | 26 ++++----------
drivers/staging/rtl8192e/rtl8192e/rtl_pm.h | 4 +--
drivers/staging/rts5208/rtsx.c | 30 +++++-----------
drivers/staging/vt6655/device_main.c | 25 ++++----------
6 files changed, 36 insertions(+), 90 deletions(-)
--
2.27.0
Drivers should not use legacy power management as they have to manage power
states and related operations, for the device, themselves. This driver was
handling them with the help of PCI helper functions like
pci_save/restore_state(), pci_enable/disable_device(), etc.
With generic PM, all essentials will be handled by the PCI core. Driver
needs to do only device-specific operations.
The driver was also using pci_enable_wake(...,..., 0) to disable wake. Use
device_wakeup_disable() instead.
Compile-tested only.
Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/staging/rts5208/rtsx.c | 30 ++++++++----------------------
1 file changed, 8 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/rts5208/rtsx.c b/drivers/staging/rts5208/rtsx.c
index be0053c795b7..6ca90694db8b 100644
--- a/drivers/staging/rts5208/rtsx.c
+++ b/drivers/staging/rts5208/rtsx.c
@@ -258,12 +258,12 @@ static int rtsx_acquire_irq(struct rtsx_dev *dev)
return 0;
}
-#ifdef CONFIG_PM
/*
* power management
*/
-static int rtsx_suspend(struct pci_dev *pci, pm_message_t state)
+static int __maybe_unused rtsx_suspend(struct device *dev_d)
{
+ struct pci_dev *pci = to_pci_dev(dev_d);
struct rtsx_dev *dev = pci_get_drvdata(pci);
struct rtsx_chip *chip;
@@ -285,10 +285,7 @@ static int rtsx_suspend(struct pci_dev *pci, pm_message_t state)
if (chip->msi_en)
pci_disable_msi(pci);
- pci_save_state(pci);
- pci_enable_wake(pci, pci_choose_state(pci, state), 1);
- pci_disable_device(pci);
- pci_set_power_state(pci, pci_choose_state(pci, state));
+ device_wakeup_enable(dev_d);
/* unlock the device pointers */
mutex_unlock(&dev->dev_mutex);
@@ -296,8 +293,9 @@ static int rtsx_suspend(struct pci_dev *pci, pm_message_t state)
return 0;
}
-static int rtsx_resume(struct pci_dev *pci)
+static int __maybe_unused rtsx_resume(struct device *dev_d)
{
+ struct pci_dev *pci = to_pci_dev(dev_d);
struct rtsx_dev *dev = pci_get_drvdata(pci);
struct rtsx_chip *chip;
@@ -309,16 +307,6 @@ static int rtsx_resume(struct pci_dev *pci)
/* lock the device pointers */
mutex_lock(&dev->dev_mutex);
- pci_set_power_state(pci, PCI_D0);
- pci_restore_state(pci);
- if (pci_enable_device(pci) < 0) {
- dev_err(&dev->pci->dev,
- "%s: pci_enable_device failed, disabling device\n",
- CR_DRIVER_NAME);
- /* unlock the device pointers */
- mutex_unlock(&dev->dev_mutex);
- return -EIO;
- }
pci_set_master(pci);
if (chip->msi_en) {
@@ -340,7 +328,6 @@ static int rtsx_resume(struct pci_dev *pci)
return 0;
}
-#endif /* CONFIG_PM */
static void rtsx_shutdown(struct pci_dev *pci)
{
@@ -999,16 +986,15 @@ static const struct pci_device_id rtsx_ids[] = {
MODULE_DEVICE_TABLE(pci, rtsx_ids);
+static SIMPLE_DEV_PM_OPS(rtsx_pm_ops, rtsx_suspend, rtsx_resume);
+
/* pci_driver definition */
static struct pci_driver rtsx_driver = {
.name = CR_DRIVER_NAME,
.id_table = rtsx_ids,
.probe = rtsx_probe,
.remove = rtsx_remove,
-#ifdef CONFIG_PM
- .suspend = rtsx_suspend,
- .resume = rtsx_resume,
-#endif
+ .driver.pm = &rtsx_pm_ops,
.shutdown = rtsx_shutdown,
};
--
2.27.0
The structure of working of PM hooks for source files is:
drivers/staging/rtl8192e/rtl8192e/rtl_pm.h : callbacks declared
drivers/staging/rtl8192e/rtl8192e/rtl_pm.c : callbacks defined
drivers/staging/rtl8192e/rtl8192e/rtl_core.c : callbacks used
Drivers should not use legacy power management as they have to manage power
states and related operations, for the device, themselves. This driver was
handling them with the help of PCI helper functions like
pci_save/restore_state(), pci_enable/disable_device(), etc.
With generic PM, all essentials will be handled by the PCI core. Driver
needs to do only device-specific operations.
The driver was also using pci_enable_wake(...,..., 0) to disable wake. Use
device_wakeup_disable() instead. Use device_set_wakeup_enable() where WOL
is decided by the value of a variable during runtime.
Compile-tested only.
Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 5 ++--
drivers/staging/rtl8192e/rtl8192e/rtl_pm.c | 26 ++++++--------------
drivers/staging/rtl8192e/rtl8192e/rtl_pm.h | 4 +--
3 files changed, 12 insertions(+), 23 deletions(-)
diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index a7cd4de65b28..dbcb8d0d9707 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -63,13 +63,14 @@ static int _rtl92e_pci_probe(struct pci_dev *pdev,
static void _rtl92e_pci_disconnect(struct pci_dev *pdev);
static irqreturn_t _rtl92e_irq(int irq, void *netdev);
+static SIMPLE_DEV_PM_OPS(rtl92e_pm_ops, rtl92e_suspend, rtl92e_resume);
+
static struct pci_driver rtl8192_pci_driver = {
.name = DRV_NAME, /* Driver name */
.id_table = rtl8192_pci_id_tbl, /* PCI_ID table */
.probe = _rtl92e_pci_probe, /* probe fn */
.remove = _rtl92e_pci_disconnect, /* remove fn */
- .suspend = rtl92e_suspend, /* PM suspend fn */
- .resume = rtl92e_resume, /* PM resume fn */
+ .driver.pm = &rtl92e_pm_ops,
};
static short _rtl92e_is_tx_queue_empty(struct net_device *dev);
diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_pm.c b/drivers/staging/rtl8192e/rtl8192e/rtl_pm.c
index cd3e17b41d6f..5575186caebd 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_pm.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_pm.c
@@ -10,9 +10,9 @@
#include "rtl_pm.h"
-int rtl92e_suspend(struct pci_dev *pdev, pm_message_t state)
+int rtl92e_suspend(struct device *dev_d)
{
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct net_device *dev = dev_get_drvdata(dev_d);
struct r8192_priv *priv = rtllib_priv(dev);
u32 ulRegRead;
@@ -46,40 +46,28 @@ int rtl92e_suspend(struct pci_dev *pdev, pm_message_t state)
out_pci_suspend:
netdev_info(dev, "WOL is %s\n", priv->rtllib->bSupportRemoteWakeUp ?
"Supported" : "Not supported");
- pci_save_state(pdev);
- pci_disable_device(pdev);
- pci_enable_wake(pdev, pci_choose_state(pdev, state),
- priv->rtllib->bSupportRemoteWakeUp ? 1 : 0);
- pci_set_power_state(pdev, pci_choose_state(pdev, state));
+ device_set_wakeup_enable(dev_d, priv->rtllib->bSupportRemoteWakeUp);
mdelay(20);
return 0;
}
-int rtl92e_resume(struct pci_dev *pdev)
+int rtl92e_resume(struct device *dev_d)
{
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct pci_dev *pdev = to_pci_dev(dev_d);
+ struct net_device *dev = dev_get_drvdata(dev_d);
struct r8192_priv *priv = rtllib_priv(dev);
- int err;
u32 val;
netdev_info(dev, "================>r8192E resume call.\n");
- pci_set_power_state(pdev, PCI_D0);
-
- err = pci_enable_device(pdev);
- if (err) {
- netdev_err(dev, "pci_enable_device failed on resume\n");
- return err;
- }
- pci_restore_state(pdev);
pci_read_config_dword(pdev, 0x40, &val);
if ((val & 0x0000ff00) != 0)
pci_write_config_dword(pdev, 0x40, val & 0xffff00ff);
- pci_enable_wake(pdev, PCI_D0, 0);
+ device_wakeup_disable(dev_d);
if (priv->polling_timer_on == 0)
rtl92e_check_rfctrl_gpio_timer(&priv->gpio_polling_timer);
diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_pm.h b/drivers/staging/rtl8192e/rtl8192e/rtl_pm.h
index e58f2bcdb1dd..fd8611495975 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_pm.h
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_pm.h
@@ -10,7 +10,7 @@
#include <linux/types.h>
#include <linux/pci.h>
-int rtl92e_suspend(struct pci_dev *dev, pm_message_t state);
-int rtl92e_resume(struct pci_dev *dev);
+int rtl92e_suspend(struct device *dev_d);
+int rtl92e_resume(struct device *dev_d);
#endif
--
2.27.0
Drivers should not use legacy power management as they have to manage power
states and related operations, for the device, themselves. This driver was
handling them with the help of PCI helper functions like
pci_save/restore_state(), pci_enable/disable_device(), etc.
With generic PM, all essentials will be handled by the PCI core. Driver
needs to do only device-specific operations.
The driver was also using pci_enable_wake(...,..., 0) to disable wake. Use
device_wakeup_disable() instead.
Compile-tested only.
Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/staging/vt6655/device_main.c | 25 +++++++------------------
1 file changed, 7 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/vt6655/device_main.c b/drivers/staging/vt6655/device_main.c
index 41cbec4134b0..76de1fd568eb 100644
--- a/drivers/staging/vt6655/device_main.c
+++ b/drivers/staging/vt6655/device_main.c
@@ -1766,48 +1766,37 @@ vt6655_probe(struct pci_dev *pcid, const struct pci_device_id *ent)
/*------------------------------------------------------------------*/
-#ifdef CONFIG_PM
-static int vt6655_suspend(struct pci_dev *pcid, pm_message_t state)
+static int __maybe_unused vt6655_suspend(struct device *dev_d)
{
- struct vnt_private *priv = pci_get_drvdata(pcid);
+ struct vnt_private *priv = dev_get_drvdata(dev_d);
unsigned long flags;
spin_lock_irqsave(&priv->lock, flags);
- pci_save_state(pcid);
-
MACbShutdown(priv);
- pci_disable_device(pcid);
-
spin_unlock_irqrestore(&priv->lock, flags);
- pci_set_power_state(pcid, pci_choose_state(pcid, state));
-
return 0;
}
-static int vt6655_resume(struct pci_dev *pcid)
+static int __maybe_unused vt6655_resume(struct device *dev_d)
{
- pci_set_power_state(pcid, PCI_D0);
- pci_enable_wake(pcid, PCI_D0, 0);
- pci_restore_state(pcid);
+ device_wakeup_disable(dev_d);
return 0;
}
-#endif
MODULE_DEVICE_TABLE(pci, vt6655_pci_id_table);
+static SIMPLE_DEV_PM_OPS(vt6655_pm_ops, vt6655_suspend, vt6655_resume);
+
static struct pci_driver device_driver = {
.name = DEVICE_NAME,
.id_table = vt6655_pci_id_table,
.probe = vt6655_probe,
.remove = vt6655_remove,
-#ifdef CONFIG_PM
- .suspend = vt6655_suspend,
- .resume = vt6655_resume,
-#endif
+ .driver.pm = &vt6655_pm_ops,
};
module_pci_driver(device_driver);
--
2.27.0
Drivers should not use legacy power management as they have to manage power
states and related operations, for the device, themselves. This driver was
handling them with the help of PCI helper functions like
pci_save/restore_state(), pci_enable/disable_device(), etc.
With generic PM, all essentials will be handled by the PCI core. Driver
needs to do only device-specific operations.
The driver was also using pci_enable_wake(...,..., 0) to disable wake. Use
device_wakeup_disable() instead.
Compile-tested only.
Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/staging/qlge/qlge_main.c | 36 ++++++++------------------------
1 file changed, 9 insertions(+), 27 deletions(-)
diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index 402edaeffe12..b6f6f681c77b 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -4763,9 +4763,9 @@ static const struct pci_error_handlers qlge_err_handler = {
.resume = qlge_io_resume,
};
-static int qlge_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused qlge_suspend(struct device *dev_d)
{
- struct net_device *ndev = pci_get_drvdata(pdev);
+ struct net_device *ndev = dev_get_drvdata(dev_d);
struct ql_adapter *qdev = netdev_priv(ndev);
int err;
@@ -4779,35 +4779,19 @@ static int qlge_suspend(struct pci_dev *pdev, pm_message_t state)
}
ql_wol(qdev);
- err = pci_save_state(pdev);
- if (err)
- return err;
-
- pci_disable_device(pdev);
-
- pci_set_power_state(pdev, pci_choose_state(pdev, state));
return 0;
}
-#ifdef CONFIG_PM
-static int qlge_resume(struct pci_dev *pdev)
+static int __maybe_unused qlge_resume(struct device *dev_d)
{
- struct net_device *ndev = pci_get_drvdata(pdev);
+ struct net_device *ndev = dev_get_drvdata(dev_d);
struct ql_adapter *qdev = netdev_priv(ndev);
int err;
- pci_set_power_state(pdev, PCI_D0);
- pci_restore_state(pdev);
- err = pci_enable_device(pdev);
- if (err) {
- netif_err(qdev, ifup, qdev->ndev, "Cannot enable PCI device from suspend\n");
- return err;
- }
pci_set_master(pdev);
- pci_enable_wake(pdev, PCI_D3hot, 0);
- pci_enable_wake(pdev, PCI_D3cold, 0);
+ device_wakeup_disable(dev_d);
if (netif_running(ndev)) {
err = ql_adapter_up(qdev);
@@ -4820,22 +4804,20 @@ static int qlge_resume(struct pci_dev *pdev)
return 0;
}
-#endif /* CONFIG_PM */
static void qlge_shutdown(struct pci_dev *pdev)
{
- qlge_suspend(pdev, PMSG_SUSPEND);
+ qlge_suspend(&pdev->dev);
}
+static SIMPLE_DEV_PM_OPS(qlge_pm_ops, qlge_suspend, qlge_resume);
+
static struct pci_driver qlge_driver = {
.name = DRV_NAME,
.id_table = qlge_pci_tbl,
.probe = qlge_probe,
.remove = qlge_remove,
-#ifdef CONFIG_PM
- .suspend = qlge_suspend,
- .resume = qlge_resume,
-#endif
+ .driver.pm = &qlge_pm_ops,
.shutdown = qlge_shutdown,
.err_handler = &qlge_err_handler
};
--
2.27.0