[Linux-kernel-mentees] [PATCH v2 0/2] realtek ethernet : remove legacy power management callbacks.

[Linux-kernel-mentees] [PATCH v2 0/2] realtek ethernet : remove legacy power management callbacks.

[Vaibhav Gupta]

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.

Vaibhav Gupta (2):
  realtek/8139too: Remove Legacy Power Management
  realtek/8139cp: Remove Legacy Power Management

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

-- 
2.26.2

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

[Linux-kernel-mentees] [PATCH v2 1/2] realtek/8139too: Remove Legacy Power Management

[Vaibhav Gupta]

Upgrade power management from legacy to generic using dev_pm_ops.

Remove "struct pci_driver.suspend" and "struct pci_driver.resume"
bindings, and add "struct pci_driver.driver.pm" .

Add "__maybe_unused" attribute to resume() and susend() callbacks
definition to suppress compiler warnings.

Generic callback requires an argument of type "struct device*". Hence,
convert it to "struct net_device*" using "dev_get_drvdata()" to use
it in the callback.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 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

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

[Linux-kernel-mentees] [PATCH v2 2/2] realtek/8139cp: Remove Legacy Power Management

[Vaibhav Gupta]

Upgrade power management from legacy to generic using dev_pm_ops.

Add "__maybe_unused" attribute to resume() and susend() callbacks
definition to suppress compiler warnings.

Generic callback requires an argument of type "struct device*". Hence,
convert it to "struct net_device*" using "dev_get_drv_data()" to use
it in the callback.

Most of the cleaning part is to remove pci_save_state(),
pci_set_power_state(), etc power management function calls.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/net/ethernet/realtek/8139cp.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
index 60d342f82fb3..4f2fb1393966 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,12 @@ 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));
-
 	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 +2088,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 +2102,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 +2110,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

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [Linux-kernel-mentees] [PATCH v2 2/2] realtek/8139cp: Remove Legacy Power Management

[Vaibhav Gupta]

On Tue, 28 Apr 2020 at 22:59, Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Tue, Apr 28, 2020 at 08:13:14PM +0530, Vaibhav Gupta wrote:
> > Upgrade power management from legacy to generic using dev_pm_ops.
> >
> > Add "__maybe_unused" attribute to resume() and susend() callbacks
> > definition to suppress compiler warnings.
> >
> > Generic callback requires an argument of type "struct device*". Hence,
> > convert it to "struct net_device*" using "dev_get_drv_data()" to use
> > it in the callback.
> >
> > Most of the cleaning part is to remove pci_save_state(),
> > pci_set_power_state(), etc power management function calls.
> >
> > Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> > ---
> >  drivers/net/ethernet/realtek/8139cp.c | 25 +++++++------------------
> >  1 file changed, 7 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
> > index 60d342f82fb3..4f2fb1393966 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,12 @@ 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);
>
> This one is a little more interesting because it relies on the driver
> state (cp->wol_enabled).  IIUC, the corresponding pci_enable_wake() in
> the generic path is in pci_prepare_to_sleep() (called from
> pci_pm_suspend_noirq()).
>
> But of course the generic path doesn't look at cp->wol_enabled.  It
> looks at device_may_wakeup(), but I don't know whether there's a
> connection between that and cp->wol_enabled.
I have tested it by just compiling it. I will try to dig a bit more deep into
this and check if it is affecting something (on the basis of code).
The final test has to done with hardware.

-- Vaibhav Gupta
>
> > -     pci_set_power_state(pdev, pci_choose_state(pdev, state));
> > -
> >       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 +2088,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 +2102,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 +2110,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
> >

Re: [Linux-kernel-mentees] [PATCH v2 2/2] realtek/8139cp: Remove Legacy Power Management

[Vaibhav Gupta]

On Tue, 28 Apr 2020 at 22:59, Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Tue, Apr 28, 2020 at 08:13:14PM +0530, Vaibhav Gupta wrote:
> > Upgrade power management from legacy to generic using dev_pm_ops.
> >
> > Add "__maybe_unused" attribute to resume() and susend() callbacks
> > definition to suppress compiler warnings.
> >
> > Generic callback requires an argument of type "struct device*". Hence,
> > convert it to "struct net_device*" using "dev_get_drv_data()" to use
> > it in the callback.
> >
> > Most of the cleaning part is to remove pci_save_state(),
> > pci_set_power_state(), etc power management function calls.
> >
> > Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> > ---
> >  drivers/net/ethernet/realtek/8139cp.c | 25 +++++++------------------
> >  1 file changed, 7 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
> > index 60d342f82fb3..4f2fb1393966 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,12 @@ 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);
>
> This one is a little more interesting because it relies on the driver
> state (cp->wol_enabled).  IIUC, the corresponding pci_enable_wake() in
> the generic path is in pci_prepare_to_sleep() (called from
> pci_pm_suspend_noirq()).
>
> But of course the generic path doesn't look at cp->wol_enabled.  It
> looks at device_may_wakeup(), but I don't know whether there's a
> connection between that and cp->wol_enabled.
I have tested it by just compiling it. I will try to dig a bit more deep into
this and check if it is affecting something (on the basis of code).
The final test has to done with hardware.

-- Vaibhav Gupta
>
> > -     pci_set_power_state(pdev, pci_choose_state(pdev, state));
> > -
> >       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 +2088,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 +2102,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 +2110,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
> >
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [Linux-kernel-mentees] [PATCH v2 2/2] realtek/8139cp: Remove Legacy Power Management

[Bjorn Helgaas]

On Tue, Apr 28, 2020 at 08:13:14PM +0530, Vaibhav Gupta wrote:
> Upgrade power management from legacy to generic using dev_pm_ops.
> 
> Add "__maybe_unused" attribute to resume() and susend() callbacks
> definition to suppress compiler warnings.
> 
> Generic callback requires an argument of type "struct device*". Hence,
> convert it to "struct net_device*" using "dev_get_drv_data()" to use
> it in the callback.
> 
> Most of the cleaning part is to remove pci_save_state(),
> pci_set_power_state(), etc power management function calls.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/net/ethernet/realtek/8139cp.c | 25 +++++++------------------
>  1 file changed, 7 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
> index 60d342f82fb3..4f2fb1393966 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,12 @@ 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);

This one is a little more interesting because it relies on the driver
state (cp->wol_enabled).  IIUC, the corresponding pci_enable_wake() in
the generic path is in pci_prepare_to_sleep() (called from
pci_pm_suspend_noirq()).

But of course the generic path doesn't look at cp->wol_enabled.  It
looks at device_may_wakeup(), but I don't know whether there's a
connection between that and cp->wol_enabled.

> -	pci_set_power_state(pdev, pci_choose_state(pdev, state));
> -
>  	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 +2088,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 +2102,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 +2110,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
> 

Re: [Linux-kernel-mentees] [PATCH v2 2/2] realtek/8139cp: Remove Legacy Power Management

[Bjorn Helgaas]

On Tue, Apr 28, 2020 at 08:13:14PM +0530, Vaibhav Gupta wrote:
> Upgrade power management from legacy to generic using dev_pm_ops.
> 
> Add "__maybe_unused" attribute to resume() and susend() callbacks
> definition to suppress compiler warnings.
> 
> Generic callback requires an argument of type "struct device*". Hence,
> convert it to "struct net_device*" using "dev_get_drv_data()" to use
> it in the callback.
> 
> Most of the cleaning part is to remove pci_save_state(),
> pci_set_power_state(), etc power management function calls.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/net/ethernet/realtek/8139cp.c | 25 +++++++------------------
>  1 file changed, 7 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
> index 60d342f82fb3..4f2fb1393966 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,12 @@ 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);

This one is a little more interesting because it relies on the driver
state (cp->wol_enabled).  IIUC, the corresponding pci_enable_wake() in
the generic path is in pci_prepare_to_sleep() (called from
pci_pm_suspend_noirq()).

But of course the generic path doesn't look at cp->wol_enabled.  It
looks at device_may_wakeup(), but I don't know whether there's a
connection between that and cp->wol_enabled.

> -	pci_set_power_state(pdev, pci_choose_state(pdev, state));
> -
>  	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 +2088,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 +2102,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 +2110,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
> 
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

[Linux-kernel-mentees] [PATCH v2 2/2] realtek/8139cp: Remove Legacy Power Management

[Vaibhav Gupta]

Upgrade power management from legacy to generic using dev_pm_ops.

Add "__maybe_unused" attribute to resume() and susend() callbacks
definition to suppress compiler warnings.

Generic callback requires an argument of type "struct device*". Hence,
convert it to "struct net_device*" using "dev_get_drv_data()" to use
it in the callback.

Most of the cleaning part is to remove pci_save_state(),
pci_set_power_state(), etc power management function calls.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/net/ethernet/realtek/8139cp.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
index 60d342f82fb3..4f2fb1393966 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,12 @@ 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));
-
 	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 +2088,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 +2102,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 +2110,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

[Linux-kernel-mentees] [PATCH v2 2/2] realtek/8139cp: Remove Legacy Power Management

[Vaibhav Gupta]

Upgrade power management from legacy to generic using dev_pm_ops.

Add "__maybe_unused" attribute to resume() and susend() callbacks
definition to suppress compiler warnings.

Generic callback requires an argument of type "struct device*". Hence,
convert it to "struct net_device*" using "dev_get_drv_data()" to use
it in the callback.

Most of the cleaning part is to remove pci_save_state(),
pci_set_power_state(), etc power management function calls.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/net/ethernet/realtek/8139cp.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
index 60d342f82fb3..4f2fb1393966 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,12 @@ 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));
-
 	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 +2088,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 +2102,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 +2110,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

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees