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

[Vaibhav Gupta <vaibhav.varodek@gmail.com>]

On Fri, 24 Apr 2020 at 09:13, Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> If you post more that one patch in a series, it's best if you include
> a cover letter with the patches being responses to the cover letter.
> In this case, it would be:
>
>   [0/2] cover letter
>   -> [1/2] realtek/8139too: Remove ...
>   -> [2/2] realtek/8139cp: Remove ...
Okay! I will do it.

--Vaibhav Gupta
>
> On Thu, Apr 23, 2020 at 06:57:59PM +0530, Vaibhav Gupta wrote:
> > 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 .
> >
> > 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..b7c98b165256 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 rtl8139_suspend(struct device *device)

> >       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);
>
> The common pattern here seems to be:
>
>   #ifdef CONFIG_PM_SLEEP
>   static int *_suspend(struct device *device) { ... }
>   static int *_resume(struct device *device) { ... }
>   #endif
>   static SIMPLE_DEV_PM_OPS(*_pm_ops, *_suspend, *_resume);
>
> So your patch looks great except that I think you should keep the
> #ifdef and convert it to CONFIG_PM_SLEEP.
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees