public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Larry Finger <Larry.Finger@lwfinger.net>
To: Alexandre Demers <alexandre.f.demers@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Silence r8712u info and add verbose option v2
Date: Tue, 12 Nov 2013 13:14:30 -0600	[thread overview]
Message-ID: <52827E16.1030001@lwfinger.net> (raw)
In-Reply-To: <1384281793-4733-1-git-send-email-alexandre.f.demers@gmail.com>

On 11/12/2013 12:43 PM, Alexandre Demers wrote:
> r8712u pollutes dmesg and logs. Silence it and add a verbose option to KConfig if we ever
> really want to hear about it.
>
> v2: keep netdev_info instead of replacing it by a KERN_NOTICE
>
> ---
>   drivers/staging/rtl8712/Kconfig        | 7 +++++++
>   drivers/staging/rtl8712/rtl871x_mlme.c | 7 ++++---
>   2 files changed, 11 insertions(+), 3 deletions(-)

Do you really want the "v2" to be part of the patch title in the git repos? I 
expect not, thus you should put it inside the []. All that stuff is stripped off 
when the patch is merged.

Similarly, do you want the v2 line in the patch description to be part of the 
permanent record? If not, move it below the --- line.

You are also missing a Signed-off-by: line. Have you read 
Documentation/SubmittingPatches? At least you should run every patch through 
scripts/checkpatch.pl before submitting it.

Finally, why do you want to add another Kconfig variable? I suggest either 
deleting the logging statement, or commenting it out. I'm not sure that this 
info would ever need logging.

I NACK this patch.

Larry



  reply	other threads:[~2013-11-12 19:14 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-12 18:43 [PATCH] Silence r8712u info and add verbose option v2 Alexandre Demers
2013-11-12 19:14 ` Larry Finger [this message]
2013-11-12 21:06   ` Greg Kroah-Hartman
2013-11-12 22:38     ` Alexandre Demers
2013-11-12 23:28       ` Larry Finger

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=52827E16.1030001@lwfinger.net \
    --to=larry.finger@lwfinger.net \
    --cc=alexandre.f.demers@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    /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