[Linux-kernel-mentees] [PATCH v1] ethernet: intel: e1000: Convert to dev_pm_ops

Bjorn Helgaas helgaas at kernel.org
Sat Apr 11 00:15:39 UTC 2020


Hi Vaibhav,

Thanks for the patch.  Minor procedural nits:

1) Use "git log --oneline
drivers/net/ethernet/intel/e1000/e1000_main.c" and match the prefix to
the convention for the file.  The e1000-specific commits all start
with "e1000: "

2) The subject should say something about the overall goal and why we
want this, so "dev_pm_ops" is a little too low-level.  I like "Convert
to generic power management", e.g.,

  e1000: Convert to generic power management

When you eventually post this to netdev, I think you should post
several patches as a series, e.g., maybe all the Intel drivers
together.  In this case it's not because they depend on each other,
but because the patches will be similar and the same set of people
will review them, so it's convenient for them to look at them all at
once.

On Fri, Apr 10, 2020 at 06:14:19PM +0530, Vaibhav Gupta wrote:
> Convert the legacy callback .suspend() and .resume()
> to the generic ones.

This should mention "power management" somehow.  I think 77b84bb306fd
("xen-platform: Convert to generic power management") is a good
template (of course, since I wrote it :)).

I have other questions, so no need to repost this until we work
through those a little bit.

Bjorn


More information about the Linux-kernel-mentees mailing list