lkml.org 
[lkml]   [2013]   [May]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 2/3] pktcdvd: Convert printk to pr_<level>
From
On Thu, May 30, 2013 at 11:57 PM, Joe Perches <joe@perches.com> wrote:
> Use a more current logging style and add messages levels
> to the logging messages.
>
> Convert bare printks to pr_cont where appropriate and
> add a simple function to emit the sense string.

Few comments below.

Why not dev_dbg wherever it's possible?

> --- a/drivers/block/pktcdvd.c
> +++ b/drivers/block/pktcdvd.c

> @@ -60,7 +62,7 @@
> #include <linux/mutex.h>
> #include <linux/slab.h>
> #include <scsi/scsi_cmnd.h>
> -l#include <scsi/scsi_ioctl.h>
> +#include <scsi/scsi_ioctl.h>

Maybe you simple could remove this typo in first patch?

> @@ -734,36 +736,37 @@ out:
> return ret;
> }
>
> +static const char *sense_key_string(__u8 index)
> +{
> + static const char *info[9] = {
> + "No sense", "Recovered error", "Not ready",
> + "Medium error", "Hardware error", "Illegal request",
> + "Unit attention", "Data protect", "Blank check"
> + };
> + if (index < 8)
> + return info[index];
> + return "INVALID";
> +}
> +
> /*
> * A generic sense dump / resolve mechanism should be implemented across
> * all ATAPI + SCSI devices.
> */
> static void pkt_dump_sense(struct packet_command *cgc)
> {
> - static char *info[9] = { "No sense", "Recovered error", "Not ready",
> - "Medium error", "Hardware error", "Illegal request",
> - "Unit attention", "Data protect", "Blank check" };
> int i;
> struct request_sense *sense = cgc->sense;
>
> - printk(DRIVER_NAME":");
> + pr_err("");
> for (i = 0; i < CDROM_PACKET_SIZE; i++)
> - printk(" %02x", cgc->cmd[i]);
> - printk(" - ");
> + pr_cont(" %02x", cgc->cmd[i]);

This one is actually %*ph.

Like: pr_info("%*ph", CDROM_PACKET_SIZE, cgc->cmd);

And what is the level of this message? debug?

--
With Best Regards,
Andy Shevchenko


\
 
 \ /
  Last update: 2013-05-31 22:41    [W:0.617 / U:0.292 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site