linux-kernel-mentees.lists.linuxfoundation.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Cc: rjw@rjwysocki.net, linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [Linux-kernel-mentees] [PATCH v1] ethernet: intel: e1000: Convert to dev_pm_ops
Date: Fri, 10 Apr 2020 19:15:39 -0500	[thread overview]
Message-ID: <20200411001539.GA209636@google.com> (raw)
In-Reply-To: <20200410124418.34640-1-vaibhavgupta40@gmail.com>

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
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

  reply	other threads:[~2020-04-11  0:15 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-10 12:44 [Linux-kernel-mentees] [PATCH v1] ethernet: intel: e1000: Convert to dev_pm_ops Vaibhav Gupta
2020-04-11  0:15 ` Bjorn Helgaas [this message]
2020-04-11 14:03   ` Vaibhav Gupta
2020-04-11  0:23 ` Bjorn Helgaas
2020-04-23  0:24   ` Bjorn Helgaas
2020-05-01 20:58 ` Bjorn Helgaas
2020-05-01 21:19   ` Kirsher, Jeffrey T
2020-05-01 21:45     ` Bjorn Helgaas
2020-05-18 15:07       ` Vaibhav Gupta
2020-05-18 15:27         ` Bjorn Helgaas
2020-05-18 15:30           ` Vaibhav Gupta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200411001539.GA209636@google.com \
    --to=helgaas@kernel.org \
    --cc=linux-kernel-mentees@lists.linuxfoundation.org \
    --cc=rjw@rjwysocki.net \
    --cc=vaibhavgupta40@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).