2020-05-18 15:05:21

by Vaibhav Gupta

[permalink] [raw]
Subject: [PATCH net-next v3 0/2] realtek ethernet : use generic power management.

The purpose of this patch series is to remove legacy power management callbacks
from realtek ethernet drivers.

The callbacks performing suspend() and resume() operations are still calling
pci_save_state(), pci_set_power_state(), etc. and handling the powermanagement
themselves, which is not recommended.

The conversion requires the removal of the those function calls and change the
callback definition accordingly.

All Changes are compile-tested only.

Vaibhav Gupta (2):
realtek/8139too: use generic power management
realtek/8139cp: use generic power management

drivers/net/ethernet/realtek/8139cp.c | 25 ++++++++-----------------
drivers/net/ethernet/realtek/8139too.c | 26 +++++++-------------------
2 files changed, 15 insertions(+), 36 deletions(-)

--
2.26.2


2020-05-18 15:05:32

by Vaibhav Gupta

[permalink] [raw]
Subject: [PATCH net-next v3 1/2] realtek/8139too: use generic power management

compile-tested only

With legacy PM hooks, it was the responsibility
of a driver to manage PCI states and also
device's power state. The generic approach is
to let PCI core handle the work.

PCI core passes "struct device*" as an argument
to the .suspend() and .resume() callbacks. As
these callabcks work with "struct net_device*",
extract it from "struct device*" using
dev_get_drv_data().

Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/net/ethernet/realtek/8139too.c | 26 +++++++-------------------
1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/realtek/8139too.c b/drivers/net/ethernet/realtek/8139too.c
index 5caeb8368eab..227139d42227 100644
--- a/drivers/net/ethernet/realtek/8139too.c
+++ b/drivers/net/ethernet/realtek/8139too.c
@@ -2603,17 +2603,13 @@ static void rtl8139_set_rx_mode (struct net_device *dev)
spin_unlock_irqrestore (&tp->lock, flags);
}

-#ifdef CONFIG_PM
-
-static int rtl8139_suspend (struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused rtl8139_suspend(struct device *device)
{
- struct net_device *dev = pci_get_drvdata (pdev);
+ struct net_device *dev = dev_get_drvdata(device);
struct rtl8139_private *tp = netdev_priv(dev);
void __iomem *ioaddr = tp->mmio_addr;
unsigned long flags;

- pci_save_state (pdev);
-
if (!netif_running (dev))
return 0;

@@ -2631,38 +2627,30 @@ static int rtl8139_suspend (struct pci_dev *pdev, pm_message_t state)

spin_unlock_irqrestore (&tp->lock, flags);

- pci_set_power_state (pdev, PCI_D3hot);
-
return 0;
}

-
-static int rtl8139_resume (struct pci_dev *pdev)
+static int __maybe_unused rtl8139_resume(struct device *device)
{
- struct net_device *dev = pci_get_drvdata (pdev);
+ struct net_device *dev = dev_get_drvdata(device);

- pci_restore_state (pdev);
if (!netif_running (dev))
return 0;
- pci_set_power_state (pdev, PCI_D0);
+
rtl8139_init_ring (dev);
rtl8139_hw_start (dev);
netif_device_attach (dev);
return 0;
}

-#endif /* CONFIG_PM */
-
+static SIMPLE_DEV_PM_OPS(rtl8139_pm_ops, rtl8139_suspend, rtl8139_resume);

static struct pci_driver rtl8139_pci_driver = {
.name = DRV_NAME,
.id_table = rtl8139_pci_tbl,
.probe = rtl8139_init_one,
.remove = rtl8139_remove_one,
-#ifdef CONFIG_PM
- .suspend = rtl8139_suspend,
- .resume = rtl8139_resume,
-#endif /* CONFIG_PM */
+ .driver.pm = &rtl8139_pm_ops,
};


--
2.26.2

2020-05-18 15:06:20

by Vaibhav Gupta

[permalink] [raw]
Subject: [PATCH net-next v3 2/2] realtek/8139cp: use generic power management

compile-tested only

With legacy PM hooks, it was the responsibility
of a driver to manage PCI states and also
device's power state. The generic approach is
to let PCI core handle the work.

The suspend callback enables/disables PCI wake
on the basis of "cp->wol_enabled" variable
which is unknown to PCI core. To utilise its
need, call device_set_wakeup_enable().

Signed-off-by: Vaibhav Gupta <[email protected]>
---
drivers/net/ethernet/realtek/8139cp.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
index 60d342f82fb3..e291e6ac40cb 100644
--- a/drivers/net/ethernet/realtek/8139cp.c
+++ b/drivers/net/ethernet/realtek/8139cp.c
@@ -2054,10 +2054,9 @@ static void cp_remove_one (struct pci_dev *pdev)
free_netdev(dev);
}

-#ifdef CONFIG_PM
-static int cp_suspend (struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused cp_suspend(struct device *device)
{
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct net_device *dev = dev_get_drvdata(device);
struct cp_private *cp = netdev_priv(dev);
unsigned long flags;

@@ -2075,16 +2074,14 @@ static int cp_suspend (struct pci_dev *pdev, pm_message_t state)

spin_unlock_irqrestore (&cp->lock, flags);

- pci_save_state(pdev);
- pci_enable_wake(pdev, pci_choose_state(pdev, state), cp->wol_enabled);
- pci_set_power_state(pdev, pci_choose_state(pdev, state));
+ device_set_wakeup_enable(device, cp->wol_enabled);

return 0;
}

-static int cp_resume (struct pci_dev *pdev)
+static int __maybe_unused cp_resume(struct device *device)
{
- struct net_device *dev = pci_get_drvdata (pdev);
+ struct net_device *dev = dev_get_drvdata(device);
struct cp_private *cp = netdev_priv(dev);
unsigned long flags;

@@ -2093,10 +2090,6 @@ static int cp_resume (struct pci_dev *pdev)

netif_device_attach (dev);

- pci_set_power_state(pdev, PCI_D0);
- pci_restore_state(pdev);
- pci_enable_wake(pdev, PCI_D0, 0);
-
/* FIXME: sh*t may happen if the Rx ring buffer is depleted */
cp_init_rings_index (cp);
cp_init_hw (cp);
@@ -2111,7 +2104,6 @@ static int cp_resume (struct pci_dev *pdev)

return 0;
}
-#endif /* CONFIG_PM */

static const struct pci_device_id cp_pci_tbl[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK, PCI_DEVICE_ID_REALTEK_8139), },
@@ -2120,15 +2112,14 @@ static const struct pci_device_id cp_pci_tbl[] = {
};
MODULE_DEVICE_TABLE(pci, cp_pci_tbl);

+static SIMPLE_DEV_PM_OPS(cp_pm_ops, cp_suspend, cp_resume);
+
static struct pci_driver cp_driver = {
.name = DRV_NAME,
.id_table = cp_pci_tbl,
.probe = cp_init_one,
.remove = cp_remove_one,
-#ifdef CONFIG_PM
- .resume = cp_resume,
- .suspend = cp_suspend,
-#endif
+ .driver.pm = &cp_pm_ops,
};

module_pci_driver(cp_driver);
--
2.26.2

2020-05-19 22:34:38

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next v3 0/2] realtek ethernet : use generic power management.

From: Vaibhav Gupta <[email protected]>
Date: Mon, 18 May 2020 20:32:12 +0530

> The purpose of this patch series is to remove legacy power management callbacks
> from realtek ethernet drivers.
>
> The callbacks performing suspend() and resume() operations are still calling
> pci_save_state(), pci_set_power_state(), etc. and handling the powermanagement
> themselves, which is not recommended.
>
> The conversion requires the removal of the those function calls and change the
> callback definition accordingly.
>
> All Changes are compile-tested only.

Series applied, thanks.

2020-06-24 16:33:26

by Vaibhav Gupta

[permalink] [raw]
Subject: Re: [PATCH net-next v3 0/2] realtek ethernet : use generic power management.

On Wed, 20 May 2020 at 04:02, David Miller <[email protected]> wrote:
>
> From: Vaibhav Gupta <[email protected]>
> Date: Mon, 18 May 2020 20:32:12 +0530
>
> > The purpose of this patch series is to remove legacy power management callbacks
> > from realtek ethernet drivers.
> >
> > The callbacks performing suspend() and resume() operations are still calling
> > pci_save_state(), pci_set_power_state(), etc. and handling the powermanagement
> > themselves, which is not recommended.
> >
> > The conversion requires the removal of the those function calls and change the
> > callback definition accordingly.
> >
> > All Changes are compile-tested only.
>
> Series applied
Thanks !
-- Vaibhav Gupta
> , thanks.