public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Joe Perches <joe@perches.com>
Cc: Johannes Schilling <of82ecuq@cip.cs.fau.de>,
	devel@driverdev.osuosl.org, Kurt Kanzenbach <shifty91@gmail.com>,
	linux-kernel@i4.cs.fau.de,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, Al Cho <acho@novell.com>,
	Amarjargal Gundjalam <amarjargal16@gmail.com>,
	Laura Lawniczak <laura.lawniczak@googlemail.com>
Subject: Re: [PATCH 3/6] keucr: migrate printk to dev_dbg/info/warn/err
Date: Fri, 7 Jun 2013 00:11:37 +0300	[thread overview]
Message-ID: <20130606211137.GC28112@mwanda> (raw)
In-Reply-To: <1370541200.2209.6.camel@joe-AO722>

On Thu, Jun 06, 2013 at 10:53:20AM -0700, Joe Perches wrote:
> On Thu, 2013-06-06 at 18:10 +0200, Johannes Schilling wrote:
> > From: Laura Lawniczak <laura.lawniczak@googlemail.com>
> []
> > diff --git a/drivers/staging/keucr/init.c b/drivers/staging/keucr/init.c
> []
> > @@ -19,13 +19,13 @@ int ENE_InitMedia(struct us_data *us)
> []
> > -	printk(KERN_INFO "MiscReg03 = %x\n", MiscReg03);
> > +	dev_info(&us->pusb_dev->dev, "MiscReg03 = %x\n", MiscReg03);
> 
> I suggest adding a few convenience macros to make
> this a bit shorter and more likely to fit on a single
> line without exceeding 80 cols.
> 
> Adding something like:
> 
> #define us_<level>(us, fmt, ...)				\
> 	dev_<level>(&us->pusb_dev->dev, fmt, ##__VA_ARGS__)
> 
> so these uses become
> 
> 	us_info(us, "MiscReg03\n", MiscReg03);
> 
> which is shorter and more readable.
> 

Wait what?  That's not a readable example.  Why are we passing the
MiscReg03 variable when there is no %d in the string?

You're a big fan of custom printk macros but it's better if people
can stick the to the normal ones if possible.  In this case, just
using a local variable is enough:


	dev_dbg(us_dev, "scsi cmd %X --- SCSIOP_ALLOW_MEDIUM_REMOVAL\n", cmd);

79 characters.

Anyway this could fixed in a later patch.

regards,
dan carpenter

  reply	other threads:[~2013-06-06 21:12 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-05  8:50 [PATCH 0/6] keucr: checkpatch fixes Johannes Schilling
2013-06-05  8:50 ` [PATCH 1/6] keucr: remove some unneccesary typedefs Johannes Schilling
2013-06-05  9:25   ` Dan Carpenter
2013-06-05  8:50 ` [PATCH 2/6] keucr: fix some camelCase warnings Johannes Schilling
2013-06-05  9:23   ` Dan Carpenter
2013-06-05  9:48     ` Dan Carpenter
2013-06-05  8:50 ` [PATCH 3/6] keucr: migrate printk to dev_dbg/info/warn/err Johannes Schilling
2013-06-05  9:32   ` Dan Carpenter
2013-06-05  8:50 ` [PATCH 4/6] keucr: make error messages less aggressive Johannes Schilling
2013-06-05  9:36   ` Dan Carpenter
2013-06-05  8:50 ` [PATCH 5/6] keucr: fix some alignment- and whitespace-problems Johannes Schilling
2013-06-05  8:50 ` [PATCH 6/6] keucr: use more specific max_t(int, Johannes Schilling
2013-06-05  9:45   ` Dan Carpenter
2013-06-06 16:10     ` [PATCH 0/6] keucr: checkpatch fixes Johannes Schilling
2013-06-06 16:10       ` [PATCH 1/6] keucr: remove some unneccesary typedefs Johannes Schilling
2013-06-06 16:10       ` [PATCH 2/6] keucr: use more specific max_t(int, Johannes Schilling
2013-06-06 16:10       ` [PATCH 3/6] keucr: migrate printk to dev_dbg/info/warn/err Johannes Schilling
2013-06-06 17:53         ` Joe Perches
2013-06-06 21:11           ` Dan Carpenter [this message]
2013-06-06 21:23             ` Joe Perches
2013-06-06 22:07               ` Dan Carpenter
2013-06-06 21:14         ` Dan Carpenter
2013-06-06 16:10       ` [PATCH 4/6] keucr: more readable and friendly error messages Johannes Schilling
2013-06-06 21:48         ` Dan Carpenter
2013-06-06 16:10       ` [PATCH 5/6] keucr: moved function to other file Johannes Schilling
2013-06-06 21:23         ` Dan Carpenter
2013-06-06 16:10       ` [PATCH 6/6] keucr: fix some alignment- and whitespace-problems Johannes Schilling
2013-06-06 18:03         ` Joe Perches
2013-06-06 21:44         ` Dan Carpenter
2013-06-06 18:47       ` [PATCH 0/6] keucr: checkpatch fixes Greg Kroah-Hartman

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=20130606211137.GC28112@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=acho@novell.com \
    --cc=amarjargal16@gmail.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=joe@perches.com \
    --cc=laura.lawniczak@googlemail.com \
    --cc=linux-kernel@i4.cs.fau.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=of82ecuq@cip.cs.fau.de \
    --cc=shifty91@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