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

Vaibhav Gupta vaibhav.varodek at gmail.com
Fri Apr 24 10:31:43 UTC 2020


On Fri, 24 Apr 2020 at 09:17, Bjorn Helgaas <helgaas at kernel.org> wrote:
>
> On Thu, Apr 23, 2020 at 06:58:01PM +0530, Vaibhav Gupta wrote:
> > Upgrade Power Management from legacy to generic using dev_pm_ops.
> >
> > Remove pci_save_sate(), pci_set_state(), etc. No need of directive
> > '#ifdef CONFIG_PM' as, if CONFIG_PM is not defined, the binding
> > will be by deafult set to NULL.
>
> s/deafult/default/
>
> I didn't quite follow this argument.  SIMPLE_DEV_PM_OPS() is defined
> to SET_SYSTEM_SLEEP_PM_OPS(), and SET_SYSTEM_SLEEP_PM_OPS() is defined
> to nothing unless CONFIG_PM_SLEEP (not CONFIG_PM).
yes, and we can find about CONFIG_PM_SLEEP inside kernel/power/kconfig
It is defined as:
config PM_SLEEP
        def_bool y
        depends on SUSPEND || HIBERNATE_CALLBACKS
        select PM
        select SRCU

Hence it is selecting CONFIG_PM. So any of the macro check works.So i
should go with
#ifded CONFIG_PM
or
#ifdef CONFIG_PM_SLEEP ?
>
> But in any case, we don't want cp_suspend() and cp_resume() to be
> compiled at all if they're not referenced.  So I think converting the
> "#ifdef CONFIG_PM" to "#ifdef CONFIG_PM_SLEEP" is the right thing.
Actually I removed it because Andy also removed them in
226e6b866d74, thought it has to be done that way.

--Vaibhav Gupta
>
> > Signed-off-by: Vaibhav Gupta <vaibhavgupta40 at 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..e2e79cf4e1fa 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 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 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 at lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees


More information about the Linux-kernel-mentees mailing list