[PATCH] Staging: comedi: ssv_snp: fix checkpatch.pl warnings

[PATCH] Staging: comedi: ssv_snp: fix checkpatch.pl warnings

[Güngör Erseymen]

Fix two checkpatch.pl warnings about printk issues by using
pr_info(...) instead of printk(KERN_INFO, ...).

Signed-off-by: Güngör Erseymen <gelurine@gmail.com>
---
 drivers/staging/comedi/drivers/ssv_dnp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ssv_dnp.c b/drivers/staging/comedi/drivers/ssv_dnp.c
index 84b9f2a..4cd0f1b 100644
--- a/drivers/staging/comedi/drivers/ssv_dnp.c
+++ b/drivers/staging/comedi/drivers/ssv_dnp.c
@@ -177,7 +177,7 @@ static int dnp_attach(struct comedi_device *dev, struct comedi_devconfig *it)
 	struct comedi_subdevice *s;
 	int ret;
 
-	printk(KERN_INFO "comedi%d: dnp: ", dev->minor);
+	pr_info("comedi%d: dnp: ", dev->minor);
 
 	dev->board_name = board->name;
 
@@ -195,7 +195,7 @@ static int dnp_attach(struct comedi_device *dev, struct comedi_devconfig *it)
 	s->insn_bits = dnp_dio_insn_bits;
 	s->insn_config = dnp_dio_insn_config;
 
-	printk("attached\n");
+	pr_info("attached\n");
 
 	/* We use the I/O ports 0x22,0x23 and 0xa3-0xa9, which are always
 	 * allocated for the primary 8259, so we don't need to allocate them
-- 
1.7.11.4

RE: [PATCH] Staging: comedi: ssv_snp: fix checkpatch.pl warnings

[H Hartley Sweeten]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2256 bytes --]

On Thursday, August 09, 2012 8:20 AM, Güngör Erseymen wrote:
> Fix two checkpatch.pl warnings about printk issues by using
> pr_info(...) instead of printk(KERN_INFO, ...).
>
> Signed-off-by: Güngör Erseymen <gelurine@gmail.com>
> ---
>  drivers/staging/comedi/drivers/ssv_dnp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Hello Güngör,

You have a typo in the subject for this patch.
"ssv_snp" should be "ssv_dnp"

>
> diff --git a/drivers/staging/comedi/drivers/ssv_dnp.c b/drivers/staging/comedi/drivers/ssv_dnp.c
> index 84b9f2a..4cd0f1b 100644
> --- a/drivers/staging/comedi/drivers/ssv_dnp.c
> +++ b/drivers/staging/comedi/drivers/ssv_dnp.c
> @@ -177,7 +177,7 @@ static int dnp_attach(struct comedi_device *dev, struct comedi_devconfig *it)
>  	struct comedi_subdevice *s;
>  	int ret;
>  
> -	printk(KERN_INFO "comedi%d: dnp: ", dev->minor);
> +	pr_info("comedi%d: dnp: ", dev->minor);
 
Where possible, fixes like this should use the dev_printk versions.
For instance, this one would be:

	dev_info(dev->class_dev, "dnp:");

But, there is a cleaner fix for this file. See below.

> 	dev->board_name = board->name;
>
> @@ -195,7 +195,7 @@ static int dnp_attach(struct comedi_device *dev, struct comedi_devconfig *it)
> 	s->insn_bits = dnp_dio_insn_bits;
> 	s->insn_config = dnp_dio_insn_config;
> 
> -	printk("attached\n");
> +	pr_info("attached\n");

There are only two printk's in this file, both in the "attach" routine.

The first one does not have a "\n" and the function could exit without
ever terminating the message.

A better fix would be to merge the two messages into one "attached" message
at the end of the function. You could also use the dev->board_name instead of
the open coded string. Something like:

	dev_info(dev->class_dev, "%s: attached\n", dev->board_name);

Also, the message should be moved so that it's the last thing that happens
before the function returns.

Regards,
Hartley

> 
>  	/* We use the I/O ports 0x22,0x23 and 0xa3-0xa9, which are always
> 	 * allocated for the primary 8259, so we don't need to allocate them
-- 
1.7.11.4

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] Staging: comedi: ssv_snp: fix checkpatch.pl warnings

[Güngör Erseymen]

On Thu, Aug 9, 2012 at 7:27 PM, H Hartley Sweeten
<hartleys@visionengravers.com> wrote:
> You have a typo in the subject for this patch.
> "ssv_snp" should be "ssv_dnp"
Hello Hartley,

I missed that typo. This is my first patch for linux kernel, i am a bit nervous.

>> diff --git a/drivers/staging/comedi/drivers/ssv_dnp.c b/drivers/staging/comedi/drivers/ssv_dnp.c
>> index 84b9f2a..4cd0f1b 100644
>> --- a/drivers/staging/comedi/drivers/ssv_dnp.c
>> +++ b/drivers/staging/comedi/drivers/ssv_dnp.c
>> @@ -177,7 +177,7 @@ static int dnp_attach(struct comedi_device *dev, struct comedi_devconfig *it)
>>       struct comedi_subdevice *s;
>>       int ret;
>>
>> -     printk(KERN_INFO "comedi%d: dnp: ", dev->minor);
>> +     pr_info("comedi%d: dnp: ", dev->minor);
>
> Where possible, fixes like this should use the dev_printk versions.
> For instance, this one would be:
>
>         dev_info(dev->class_dev, "dnp:");
>
> But, there is a cleaner fix for this file. See below.
>
>>       dev->board_name = board->name;
>>
>> @@ -195,7 +195,7 @@ static int dnp_attach(struct comedi_device *dev, struct comedi_devconfig *it)
>>       s->insn_bits = dnp_dio_insn_bits;
>>       s->insn_config = dnp_dio_insn_config;
>>
>> -     printk("attached\n");
>> +     pr_info("attached\n");
>
> There are only two printk's in this file, both in the "attach" routine.
>
> The first one does not have a "\n" and the function could exit without
> ever terminating the message.
>
> A better fix would be to merge the two messages into one "attached" message
> at the end of the function. You could also use the dev->board_name instead of
> the open coded string. Something like:
>
>         dev_info(dev->class_dev, "%s: attached\n", dev->board_name);
>
> Also, the message should be moved so that it's the last thing that happens
> before the function returns.

Thanks for advice, i will send a new patch with changes.