[PATCH] Staging: comedi: fix printk issue in adv_pci_dio.c

[PATCH] Staging: comedi: fix printk issue in adv_pci_dio.c

[Ravishankar]

From: Ravishankar <ravi.shankar@greenturtles.in>

This is a patch to the adv_pci_dio.c file that fixes up a printk warning found by the checkpatch.pl tool

Signed-off-by: Ravishankar <ravishankarkm32@gmail.com>
---
 drivers/staging/comedi/drivers/adv_pci_dio.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adv_pci_dio.c b/drivers/staging/comedi/drivers/adv_pci_dio.c
index d23799b..1d2261f 100644
--- a/drivers/staging/comedi/drivers/adv_pci_dio.c
+++ b/drivers/staging/comedi/drivers/adv_pci_dio.c
@@ -1106,11 +1106,11 @@ static int pci_dio_attach(struct comedi_device *dev,
 	unsigned long iobase;
 	struct pci_dev *pcidev = NULL;
 
-	printk("comedi%d: adv_pci_dio: ", dev->minor);
+	printk(KERN_INFO "comedi%d: adv_pci_dio: ", dev->minor);
 
 	ret = alloc_private(dev, sizeof(struct pci_dio_private));
 	if (ret < 0) {
-		printk(", Error: Cann't allocate private memory!\n");
+		printk(KERN_CONT ", Error: Cann't allocate private memory!\n");
 		return -ENOMEM;
 	}
 
@@ -1140,19 +1140,19 @@ static int pci_dio_attach(struct comedi_device *dev,
 	}
 
 	if (!dev->board_ptr) {
-		printk(", Error: Requested type of the card was not found!\n");
+		printk(KERN_ERR ", Error: Requested type of the card was not found!\n");
 		return -EIO;
 	}
 
 	if (comedi_pci_enable(pcidev, driver_pci_dio.driver_name)) {
 		printk
-		    (", Error: Can't enable PCI device and request regions!\n");
+		(KERN_ERR ", Error: Can't enable PCI device and request "
+		"regions!\n");
 		return -EIO;
 	}
 	iobase = pci_resource_start(pcidev, this_board->main_pci_region);
-	printk(", b:s:f=%d:%d:%d, io=0x%4lx",
-	       pcidev->bus->number, PCI_SLOT(pcidev->devfn),
-	       PCI_FUNC(pcidev->devfn), iobase);
+	printk(KERN_INFO ", b:s:f=%d:%d:%d, io=0x%4lx",
+	       pcidev->bus->number, PCI_SLOT(pcidev->devfn),
+	       PCI_FUNC(pcidev->devfn), iobase);
 
 	dev->iobase = iobase;
 	dev->board_name = this_board->name;
@@ -1178,11 +1178,11 @@ static int pci_dio_attach(struct comedi_device *dev,
 
 	ret = alloc_subdevices(dev, n_subdevices);
 	if (ret < 0) {
-		printk(", Error: Cann't allocate subdevice memory!\n");
+		printk(KERN_CONT ", Error: Cann't allocate subdevice memory!\n");
 		return ret;
 	}
 
-	printk(".\n");
+	printk(KERN_CONT ".\n");
 
 	subdev = 0;
 
-- 
1.6.5.2

Re: [PATCH] Staging: comedi: fix printk issue in adv_pci_dio.c

[Joe Perches]

On Wed, 2011-07-27 at 12:01 +0530, Ravishankar wrote:
> From: Ravishankar <ravi.shankar@greenturtles.in>
> 
> This is a patch to the adv_pci_dio.c file that fixes up a printk warning found by the checkpatch.pl tool
> 
> Signed-off-by: Ravishankar <ravishankarkm32@gmail.com>
> ---
>  drivers/staging/comedi/drivers/adv_pci_dio.c |   18 +++++++++---------
>  1 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/comedi/drivers/adv_pci_dio.c b/drivers/staging/comedi/drivers/adv_pci_dio.c
> index d23799b..1d2261f 100644
> --- a/drivers/staging/comedi/drivers/adv_pci_dio.c
> +++ b/drivers/staging/comedi/drivers/adv_pci_dio.c
> @@ -1106,11 +1106,11 @@ static int pci_dio_attach(struct comedi_device *dev,
>  	unsigned long iobase;
>  	struct pci_dev *pcidev = NULL;
>  
> -	printk("comedi%d: adv_pci_dio: ", dev->minor);
> +	printk(KERN_INFO "comedi%d: adv_pci_dio: ", dev->minor);
>  
>  	ret = alloc_private(dev, sizeof(struct pci_dio_private));
>  	if (ret < 0) {
> -		printk(", Error: Cann't allocate private memory!\n");
> +		printk(KERN_CONT ", Error: Cann't allocate private memory!\n");
>  		return -ENOMEM;
>  	}
>  
> @@ -1140,19 +1140,19 @@ static int pci_dio_attach(struct comedi_device *dev,
>  	}
>  
>  	if (!dev->board_ptr) {
> -		printk(", Error: Requested type of the card was not found!\n");
> +		printk(KERN_ERR ", Error: Requested type of the card was not found!\n");
>  		return -EIO;
>  	}
>  
>  	if (comedi_pci_enable(pcidev, driver_pci_dio.driver_name)) {
>  		printk
> -		    (", Error: Can't enable PCI device and request regions!\n");
> +		(KERN_ERR ", Error: Can't enable PCI device and request "
> +		"regions!\n");
>  		return -EIO;
>  	}
>  	iobase = pci_resource_start(pcidev, this_board->main_pci_region);
> -	printk(", b:s:f=%d:%d:%d, io=0x%4lx",
> -	       pcidev->bus->number, PCI_SLOT(pcidev->devfn),
> -	       PCI_FUNC(pcidev->devfn), iobase);
> +	printk(KERN_INFO ", b:s:f=%d:%d:%d, io=0x%4lx",
> +	       pcidev->bus->number, PCI_SLOT(pcidev->devfn),
> +	       PCI_FUNC(pcidev->devfn), iobase);
>  
>  	dev->iobase = iobase;
>  	dev->board_name = this_board->name;
> @@ -1178,11 +1178,11 @@ static int pci_dio_attach(struct comedi_device *dev,
>  
>  	ret = alloc_subdevices(dev, n_subdevices);
>  	if (ret < 0) {
> -		printk(", Error: Cann't allocate subdevice memory!\n");
> +		printk(KERN_CONT ", Error: Cann't allocate subdevice memory!\n");
>  		return ret;
>  	}
>  
> -	printk(".\n");
> +	printk(KERN_CONT ".\n");
>  
>  	subdev = 0;
>  

Ravishankar, you _really_ aren't looking too hard when
you do these changes.  Your error rate is _quite_ high.

All of these changes are in the same function and are
continuations of the initial printk.  Every one of these
after the first one should use KERN_CONT.

_Please_ look harder before submitting these changes.

Otherwise, I'm going to have to agree with Dan Carpenter.

I'm sure you can do these sorts of trivial changes, but
you absolutely _must_ lower your error rate, otherwise
you work will be ignored and you will lose whatever
reputation you might want to establish within this
community.

Best, Joe Perches

[PATCH v2] Staging: comedi: fix printk issue in adv_pci_dio.c

[Ravishankar]

From: Ravishankar <ravi.shankar@greenturtles.in>

This is a patch to the adv_pci_dio.c file that fixes up a printk warning found by the checkpatch.pl tool

Signed-off-by: Ravishankar <ravishankarkm32@gmail.com>
---
 drivers/staging/comedi/drivers/adv_pci_dio.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adv_pci_dio.c b/drivers/staging/comedi/drivers/adv_pci_dio.c
index d23799b..1d2261f 100644
--- a/drivers/staging/comedi/drivers/adv_pci_dio.c
+++ b/drivers/staging/comedi/drivers/adv_pci_dio.c
@@ -1106,11 +1106,11 @@ static int pci_dio_attach(struct comedi_device *dev,
 	unsigned long iobase;
 	struct pci_dev *pcidev = NULL;
 
-	printk("comedi%d: adv_pci_dio: ", dev->minor);
+	printk(KERN_INFO "comedi%d: adv_pci_dio: ", dev->minor);
 
 	ret = alloc_private(dev, sizeof(struct pci_dio_private));
 	if (ret < 0) {
-		printk(", Error: Cann't allocate private memory!\n");
+		printk(KERN_CONT ", Error: Cann't allocate private memory!\n");
 		return -ENOMEM;
 	}
 
@@ -1140,19 +1140,19 @@ static int pci_dio_attach(struct comedi_device *dev,
 	}
 
 	if (!dev->board_ptr) {
-		printk(", Error: Requested type of the card was not found!\n");
+		printk(KERN_WARNING ", Error: Requested type of the card was not "
+		"found!\n");
 		return -EIO;
 	}
 
 	if (comedi_pci_enable(pcidev, driver_pci_dio.driver_name)) {
 		printk
-		    (", Error: Can't enable PCI device and request regions!\n");
+		(KERN_WARNING ", Error: Can't enable PCI device and request "
+		"regions!\n");
 		return -EIO;
 	}
 	iobase = pci_resource_start(pcidev, this_board->main_pci_region);
-	printk(", b:s:f=%d:%d:%d, io=0x%4lx",
-	       pcidev->bus->number, PCI_SLOT(pcidev->devfn),
-	       PCI_FUNC(pcidev->devfn), iobase);
+	printk(KERN_CONT ", b:s:f=%d:%d:%d, io=0x%4lx",
+	       pcidev->bus->number, PCI_SLOT(pcidev->devfn),
+	       PCI_FUNC(pcidev->devfn), iobase);

 	dev->iobase = iobase;
 	dev->board_name = this_board->name;
@@ -1178,11 +1178,11 @@ static int pci_dio_attach(struct comedi_device *dev,
 
 	ret = alloc_subdevices(dev, n_subdevices);
 	if (ret < 0) {
-		printk(", Error: Cann't allocate subdevice memory!\n");
+		printk(KERN_CONT ", Error: Cann't allocate subdevice memory!\n");
 		return ret;
 	}
 
-	printk(".\n");
+	printk(KERN_CONT ".\n");
 
 	subdev = 0;
 
-- 
1.6.5.2

Re: [PATCH v2] Staging: comedi: fix printk issue in adv_pci_dio.c

[Jesper Juhl]

On Wed, 27 Jul 2011, Ravishankar wrote:

> From: Ravishankar <ravi.shankar@greenturtles.in>
> 
> This is a patch to the adv_pci_dio.c file that fixes up a printk warning found by the checkpatch.pl tool
> 
> Signed-off-by: Ravishankar <ravishankarkm32@gmail.com>
> ---
>  drivers/staging/comedi/drivers/adv_pci_dio.c |   18 +++++++++---------
>  1 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/comedi/drivers/adv_pci_dio.c b/drivers/staging/comedi/drivers/adv_pci_dio.c
> index d23799b..1d2261f 100644
> --- a/drivers/staging/comedi/drivers/adv_pci_dio.c
> +++ b/drivers/staging/comedi/drivers/adv_pci_dio.c
> @@ -1106,11 +1106,11 @@ static int pci_dio_attach(struct comedi_device *dev,
>  	unsigned long iobase;
>  	struct pci_dev *pcidev = NULL;
>  
> -	printk("comedi%d: adv_pci_dio: ", dev->minor);
> +	printk(KERN_INFO "comedi%d: adv_pci_dio: ", dev->minor);
>  

This printk() is used both for printing out information in the case of 
success, in which case the KERN_INFO level is fine. But it is also used to 
print out error messages in case of failure, in which case KERN_WARNING 
would probably be better. So I'm wondering if it wouldn't be better to 
restructure the code so that the printing of error messages and success 
info becomes two seperate printk()'s each with the apropriate level.


>  	ret = alloc_private(dev, sizeof(struct pci_dio_private));
>  	if (ret < 0) {
> -		printk(", Error: Cann't allocate private memory!\n");
> +		printk(KERN_CONT ", Error: Cann't allocate private memory!\n");

Might as well fix the spelling error ( s/Cann't/Can't/ ) while you are 
changing the line anyway.


>  		return -ENOMEM;
>  	}
>  
> @@ -1140,19 +1140,19 @@ static int pci_dio_attach(struct comedi_device *dev,
>  	}
>  
>  	if (!dev->board_ptr) {
> -		printk(", Error: Requested type of the card was not found!\n");
> +		printk(KERN_WARNING ", Error: Requested type of the card was not "
> +		"found!\n");

As Joe Perches already pointed out to you, this is a continuation of the 
first printk() and should be using KERN_CONT.


>  		return -EIO;
>  	}
>  
>  	if (comedi_pci_enable(pcidev, driver_pci_dio.driver_name)) {
>  		printk
> -		    (", Error: Can't enable PCI device and request regions!\n");
> +		(KERN_WARNING ", Error: Can't enable PCI device and request "
> +		"regions!\n");

This one as well. And it has got to be possible to find a less hideous way 
to break that line..

what about:

  printk(KERN_CONT
         ", Error: Can't enable PCI device and request regions!\n");

? If it even needs to be broken at all (the 80 column rule is not fixed in 
stone).


>  		return -EIO;
>  	}
>  	iobase = pci_resource_start(pcidev, this_board->main_pci_region);
> -	printk(", b:s:f=%d:%d:%d, io=0x%4lx",
> -	       pcidev->bus->number, PCI_SLOT(pcidev->devfn),
> -	       PCI_FUNC(pcidev->devfn), iobase);
> +	printk(KERN_CONT ", b:s:f=%d:%d:%d, io=0x%4lx",
> +	       pcidev->bus->number, PCI_SLOT(pcidev->devfn),
> +	       PCI_FUNC(pcidev->devfn), iobase);
> 
>  	dev->iobase = iobase;
>  	dev->board_name = this_board->name;
> @@ -1178,11 +1178,11 @@ static int pci_dio_attach(struct comedi_device *dev,
>  
>  	ret = alloc_subdevices(dev, n_subdevices);
>  	if (ret < 0) {
> -		printk(", Error: Cann't allocate subdevice memory!\n");
> +		printk(KERN_CONT ", Error: Cann't allocate subdevice memory!\n");
>  		return ret;
>  	}
>  
> -	printk(".\n");
> +	printk(KERN_CONT ".\n");
>  
>  	subdev = 0;
>  
 
Please take the feedback people give you serious. Joe kindly pointed out 
your mistakes the last time you posted this and yet your next patch has 
the exact same mistakes.

Read through your patches before you send them and double check each and 
every change - then check it again.

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

[PATCH v2] Staging: comedi: fix printk issue in adv_pci_dio.c

[ravi shankar]

Hi,

      Jesper as you told it would be better to make one
printk(KERN_ERR...) for all levels instead of two. What is your
opinion?

Thanks & Regards
 Ravishankar

On Wed, Jul 27, 2011 at 6:05 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> On Wed, 27 Jul 2011, Ravishankar wrote:
>
>> From: Ravishankar <ravi.shankar@greenturtles.in>
>>
>> This is a patch to the adv_pci_dio.c file that fixes up a printk warning found by the checkpatch.pl tool
>>
>> Signed-off-by: Ravishankar <ravishankarkm32@gmail.com>
>> ---
>>  drivers/staging/comedi/drivers/adv_pci_dio.c |   18 +++++++++---------
>>  1 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/adv_pci_dio.c b/drivers/staging/comedi/drivers/adv_pci_dio.c
>> index d23799b..1d2261f 100644
>> --- a/drivers/staging/comedi/drivers/adv_pci_dio.c
>> +++ b/drivers/staging/comedi/drivers/adv_pci_dio.c
>> @@ -1106,11 +1106,11 @@ static int pci_dio_attach(struct comedi_device *dev,
>>       unsigned long iobase;
>>       struct pci_dev *pcidev = NULL;
>>
>> -     printk("comedi%d: adv_pci_dio: ", dev->minor);
>> +     printk(KERN_INFO "comedi%d: adv_pci_dio: ", dev->minor);
>>
>
> This printk() is used both for printing out information in the case of
> success, in which case the KERN_INFO level is fine. But it is also used to
> print out error messages in case of failure, in which case KERN_WARNING
> would probably be better. So I'm wondering if it wouldn't be better to
> restructure the code so that the printing of error messages and success
> info becomes two seperate printk()'s each with the apropriate level.
>
>
>>       ret = alloc_private(dev, sizeof(struct pci_dio_private));
>>       if (ret < 0) {
>> -             printk(", Error: Cann't allocate private memory!\n");
>> +             printk(KERN_CONT ", Error: Cann't allocate private memory!\n");
>
> Might as well fix the spelling error ( s/Cann't/Can't/ ) while you are
> changing the line anyway.
>
>
>>               return -ENOMEM;
>>       }
>>
>> @@ -1140,19 +1140,19 @@ static int pci_dio_attach(struct comedi_device *dev,
>>       }
>>
>>       if (!dev->board_ptr) {
>> -             printk(", Error: Requested type of the card was not found!\n");
>> +             printk(KERN_WARNING ", Error: Requested type of the card was not "
>> +             "found!\n");
>
> As Joe Perches already pointed out to you, this is a continuation of the
> first printk() and should be using KERN_CONT.
>
>
>>               return -EIO;
>>       }
>>
>>       if (comedi_pci_enable(pcidev, driver_pci_dio.driver_name)) {
>>               printk
>> -                 (", Error: Can't enable PCI device and request regions!\n");
>> +             (KERN_WARNING ", Error: Can't enable PCI device and request "
>> +             "regions!\n");
>
> This one as well. And it has got to be possible to find a less hideous way
> to break that line..
>
> what about:
>
>  printk(KERN_CONT
>         ", Error: Can't enable PCI device and request regions!\n");
>
> ? If it even needs to be broken at all (the 80 column rule is not fixed in
> stone).
>
>
>>               return -EIO;
>>       }
>>       iobase = pci_resource_start(pcidev, this_board->main_pci_region);
>> -     printk(", b:s:f=%d:%d:%d, io=0x%4lx",
>> -            pcidev->bus->number, PCI_SLOT(pcidev->devfn),
>> -            PCI_FUNC(pcidev->devfn), iobase);
>> +     printk(KERN_CONT ", b:s:f=%d:%d:%d, io=0x%4lx",
>> +            pcidev->bus->number, PCI_SLOT(pcidev->devfn),
>> +            PCI_FUNC(pcidev->devfn), iobase);
>>
>>       dev->iobase = iobase;
>>       dev->board_name = this_board->name;
>> @@ -1178,11 +1178,11 @@ static int pci_dio_attach(struct comedi_device *dev,
>>
>>       ret = alloc_subdevices(dev, n_subdevices);
>>       if (ret < 0) {
>> -             printk(", Error: Cann't allocate subdevice memory!\n");
>> +             printk(KERN_CONT ", Error: Cann't allocate subdevice memory!\n");
>>               return ret;
>>       }
>>
>> -     printk(".\n");
>> +     printk(KERN_CONT ".\n");
>>
>>       subdev = 0;
>>
>
> Please take the feedback people give you serious. Joe kindly pointed out
> your mistakes the last time you posted this and yet your next patch has
> the exact same mistakes.
>
> Read through your patches before you send them and double check each and
> every change - then check it again.
>
> --
> Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
> Don't top-post http://www.catb.org/jargon/html/T/top-post.html
> Plain text mails only, please.
>
>

Re: [PATCH v2] Staging: comedi: fix printk issue in adv_pci_dio.c

[Jesper Juhl]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 5073 bytes --]

On Thu, 28 Jul 2011, ravi shankar wrote:

> Hi,
> 
>       Jesper as you told it would be better to make one
> printk(KERN_ERR...) for all levels instead of two. What is your
> opinion?
> 

That's not what I said. Read my mail again.

I said that in my opinion the failure message(s) and the success message 
should probably be two different printk()'s.

But regardless of whether you keep it as-is or not, the other comments 
still need to be addressed.


> Thanks & Regards
>  Ravishankar
> 
> On Wed, Jul 27, 2011 at 6:05 PM, Jesper Juhl <jj@chaosbits.net> wrote:
> > On Wed, 27 Jul 2011, Ravishankar wrote:
> >
> >> From: Ravishankar <ravi.shankar@greenturtles.in>
> >>
> >> This is a patch to the adv_pci_dio.c file that fixes up a printk warning found by the checkpatch.pl tool
> >>
> >> Signed-off-by: Ravishankar <ravishankarkm32@gmail.com>
> >> ---
> >>  drivers/staging/comedi/drivers/adv_pci_dio.c |   18 +++++++++---------
> >>  1 files changed, 9 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/staging/comedi/drivers/adv_pci_dio.c b/drivers/staging/comedi/drivers/adv_pci_dio.c
> >> index d23799b..1d2261f 100644
> >> --- a/drivers/staging/comedi/drivers/adv_pci_dio.c
> >> +++ b/drivers/staging/comedi/drivers/adv_pci_dio.c
> >> @@ -1106,11 +1106,11 @@ static int pci_dio_attach(struct comedi_device *dev,
> >>       unsigned long iobase;
> >>       struct pci_dev *pcidev = NULL;
> >>
> >> -     printk("comedi%d: adv_pci_dio: ", dev->minor);
> >> +     printk(KERN_INFO "comedi%d: adv_pci_dio: ", dev->minor);
> >>
> >
> > This printk() is used both for printing out information in the case of
> > success, in which case the KERN_INFO level is fine. But it is also used to
> > print out error messages in case of failure, in which case KERN_WARNING
> > would probably be better. So I'm wondering if it wouldn't be better to
> > restructure the code so that the printing of error messages and success
> > info becomes two seperate printk()'s each with the apropriate level.
> >
> >
> >>       ret = alloc_private(dev, sizeof(struct pci_dio_private));
> >>       if (ret < 0) {
> >> -             printk(", Error: Cann't allocate private memory!\n");
> >> +             printk(KERN_CONT ", Error: Cann't allocate private memory!\n");
> >
> > Might as well fix the spelling error ( s/Cann't/Can't/ ) while you are
> > changing the line anyway.
> >
> >
> >>               return -ENOMEM;
> >>       }
> >>
> >> @@ -1140,19 +1140,19 @@ static int pci_dio_attach(struct comedi_device *dev,
> >>       }
> >>
> >>       if (!dev->board_ptr) {
> >> -             printk(", Error: Requested type of the card was not found!\n");
> >> +             printk(KERN_WARNING ", Error: Requested type of the card was not "
> >> +             "found!\n");
> >
> > As Joe Perches already pointed out to you, this is a continuation of the
> > first printk() and should be using KERN_CONT.
> >
> >
> >>               return -EIO;
> >>       }
> >>
> >>       if (comedi_pci_enable(pcidev, driver_pci_dio.driver_name)) {
> >>               printk
> >> -                 (", Error: Can't enable PCI device and request regions!\n");
> >> +             (KERN_WARNING ", Error: Can't enable PCI device and request "
> >> +             "regions!\n");
> >
> > This one as well. And it has got to be possible to find a less hideous way
> > to break that line..
> >
> > what about:
> >
> >  printk(KERN_CONT
> >         ", Error: Can't enable PCI device and request regions!\n");
> >
> > ? If it even needs to be broken at all (the 80 column rule is not fixed in
> > stone).
> >
> >
> >>               return -EIO;
> >>       }
> >>       iobase = pci_resource_start(pcidev, this_board->main_pci_region);
> >> -     printk(", b:s:f=%d:%d:%d, io=0x%4lx",
> >> -            pcidev->bus->number, PCI_SLOT(pcidev->devfn),
> >> -            PCI_FUNC(pcidev->devfn), iobase);
> >> +     printk(KERN_CONT ", b:s:f=%d:%d:%d, io=0x%4lx",
> >> +            pcidev->bus->number, PCI_SLOT(pcidev->devfn),
> >> +            PCI_FUNC(pcidev->devfn), iobase);
> >>
> >>       dev->iobase = iobase;
> >>       dev->board_name = this_board->name;
> >> @@ -1178,11 +1178,11 @@ static int pci_dio_attach(struct comedi_device *dev,
> >>
> >>       ret = alloc_subdevices(dev, n_subdevices);
> >>       if (ret < 0) {
> >> -             printk(", Error: Cann't allocate subdevice memory!\n");
> >> +             printk(KERN_CONT ", Error: Cann't allocate subdevice memory!\n");
> >>               return ret;
> >>       }
> >>
> >> -     printk(".\n");
> >> +     printk(KERN_CONT ".\n");
> >>
> >>       subdev = 0;
> >>
> >
> > Please take the feedback people give you serious. Joe kindly pointed out
> > your mistakes the last time you posted this and yet your next patch has
> > the exact same mistakes.
> >
> > Read through your patches before you send them and double check each and
> > every change - then check it again.
> >


-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

[PATCH v2] Staging: comedi: fix printk issue in adv_pci_dio.c

[Ravishankar]

From: Ravishankar <ravi.shankar@greenturtles.in>

This is a patch to the adv_pci_dio.c file that fixes up a printk warning found by the checkpatch.pl tool

Signed-off-by: Ravishankar <ravishankarkm32@gmail.com>
---
 drivers/staging/comedi/drivers/adv_pci_dio.c |   26 +++++++++++++++-----------
 1 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adv_pci_dio.c b/drivers/staging/comedi/drivers/adv_pci_dio.c
index d23799b..fe66d5f 100644
--- a/drivers/staging/comedi/drivers/adv_pci_dio.c
+++ b/drivers/staging/comedi/drivers/adv_pci_dio.c
@@ -422,7 +422,7 @@ struct pci_dio_private {
 	unsigned short IDIFiltrHigh[8];	/*  IDI's filter value high signal */
 };
 
-static struct pci_dio_private *pci_priv = NULL;	/* list of allocated cards */
+static struct pci_dio_private *pci_priv;	/* list of allocated cards */
 
 #define devpriv ((struct pci_dio_private *)dev->private)
 #define this_board ((const struct dio_boardtype *)dev->board_ptr)
@@ -1106,14 +1106,15 @@ static int pci_dio_attach(struct comedi_device *dev,
 	unsigned long iobase;
 	struct pci_dev *pcidev = NULL;
 
-	printk("comedi%d: adv_pci_dio: ", dev->minor);
+	printk(KERN_INFO "comedi%d: adv_pci_dio: ", dev->minor);
 
 	ret = alloc_private(dev, sizeof(struct pci_dio_private));
 	if (ret < 0) {
-		printk(", Error: Cann't allocate private memory!\n");
+		printk(KERN_WARNING "Error: Can't allocate private memory!\n");
 		return -ENOMEM;
 	}
 
+	printk(KERN_CONT ", Successfully allocated private memory\n");
 	for_each_pci_dev(pcidev) {
 		/*  loop through cards supported by this driver */
 		for (i = 0; i < n_boardtypes; ++i) {
@@ -1140,19 +1141,21 @@ static int pci_dio_attach(struct comedi_device *dev,
 	}
 
 	if (!dev->board_ptr) {
-		printk(", Error: Requested type of the card was not found!\n");
+		printk(KERN_WARNING "Error: Requested type of the card was not found!\n");
 		return -EIO;
 	}
 
+	printk(KERN_CONT ", Requested type of card was found\n");
 	if (comedi_pci_enable(pcidev, driver_pci_dio.driver_name)) {
-		printk
-		    (", Error: Can't enable PCI device and request regions!\n");
+		printk(KERN_WARNING
+		"Error: Can't enable PCI device and request regions!\n");
 		return -EIO;
 	}
+	printk(KERN_CONT "Enabled PCI device and request reqions\n");
 	iobase = pci_resource_start(pcidev, this_board->main_pci_region);
-	printk(", b:s:f=%d:%d:%d, io=0x%4lx",
-	       pcidev->bus->number, PCI_SLOT(pcidev->devfn),
-	       PCI_FUNC(pcidev->devfn), iobase);
+	printk(KERN_CONT ", b:s:f=%d:%d:%d, io=0x%4lx",
+		pcidev->bus->number, PCI_SLOT(pcidev->devfn),
+		PCI_FUNC(pcidev->devfn), iobase);
 
 	dev->iobase = iobase;
 	dev->board_name = this_board->name;
@@ -1178,11 +1181,12 @@ static int pci_dio_attach(struct comedi_device *dev,
 
 	ret = alloc_subdevices(dev, n_subdevices);
 	if (ret < 0) {
-		printk(", Error: Cann't allocate subdevice memory!\n");
+		printk(KERN_WARNING "Error: Can't allocate subdevice memory!\n");
 		return ret;
 	}
 
-	printk(".\n");
+	printk(KERN_CONT ", Successfully allocated subdevice memory\n");
+	printk(KERN_CONT ".\n");
 
 	subdev = 0;
 
-- 
1.6.5.2