[PATCH 1/2 v2] Staging: comedi: fix printk() issue in adv_pci1710.c

[PATCH 1/2 v2] Staging: comedi: fix printk() issue in adv_pci1710.c

[Ravishankar]

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

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

Signed-off-by: Ravishankar <ravishankarkm32@gmail.com>
---

 KERN_INFO is replaced by KERN_ERR. 


 drivers/staging/comedi/drivers/adv_pci1710.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c b/drivers/staging/comedi/drivers/adv_pci1710.c
index fd71cc6..093b9e6 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -1396,14 +1396,14 @@ static int pci1710_attach(struct comedi_device *dev,
 	int i;
 	int board_index;
 
-	printk("comedi%d: adv_pci1710: ", dev->minor);
+	printk(KERN_INFO "comedi%d: adv_pci1710: ", dev->minor);
 
 	opt_bus = it->options[0];
 	opt_slot = it->options[1];
 
 	ret = alloc_private(dev, sizeof(struct pci1710_private));
 	if (ret < 0) {
-		printk(" - Allocation failed!\n");
+		printk(KERN_ERR " - Allocation failed!\n");
 		return -ENOMEM;
 	}
 
@@ -1451,10 +1451,10 @@ static int pci1710_attach(struct comedi_device *dev,
 
 	if (!pcidev) {
 		if (opt_bus || opt_slot) {
-			printk(" - Card at b:s %d:%d %s\n",
+			printk(KERN_ERR " - Card at b:s %d:%d %s\n",
 			       opt_bus, opt_slot, errstr);
 		} else {
-			printk(" - Card %s\n", errstr);
+			printk(KERN_ERR " - Card %s\n", errstr);
 		}
 		return -EIO;
 	}
@@ -1465,7 +1465,7 @@ static int pci1710_attach(struct comedi_device *dev,
 	irq = pcidev->irq;
 	iobase = pci_resource_start(pcidev, 2);
 
-	printk(", b:s:f=%d:%d:%d, io=0x%4lx", pci_bus, pci_slot, pci_func,
+	printk(KERN_INFO ", b:s:f=%d:%d:%d, io=0x%4lx", pci_bus, pci_slot,
+	  pci_func, iobase);
 
 	dev->iobase = iobase;
@@ -1487,7 +1487,7 @@ static int pci1710_attach(struct comedi_device *dev,
 
 	ret = alloc_subdevices(dev, n_subdevices);
 	if (ret < 0) {
-		printk(" - Allocation failed!\n");
+		printk(KERN_ERR " - Allocation failed!\n");
 		return ret;
 	}
 
@@ -1499,14 +1499,14 @@ static int pci1710_attach(struct comedi_device *dev,
 					IRQF_SHARED, "Advantech PCI-1710",
 					dev)) {
 				printk
-				    (", unable to allocate IRQ %d, DISABLING IT",
+				    (KERN_CONT ", unable to allocate IRQ %d, DISABLING IT",
 				     irq);
 				irq = 0;	/* Can't use IRQ */
 			} else {
-				printk(", irq=%u", irq);
+				printk(KERN_CONT ", irq=%u", irq);
 			}
 		} else {
-			printk(", IRQ disabled");
+			printk(KERN_CONT ", IRQ disabled");
 		}
 	} else {
 		irq = 0;
@@ -1514,7 +1514,7 @@ static int pci1710_attach(struct comedi_device *dev,
 
 	dev->irq = irq;
 
-	printk(".\n");
+	printk(KERN_CONT ".\n");
 
 	subdev = 0;
 
-- 
1.6.5.2

Re: [PATCH 1/2 v2] Staging: comedi: fix printk() issue in adv_pci1710.c

[Joe Perches]

On Tue, 2011-07-19 at 09:22 +0530, Ravishankar wrote:
> From: Ravishankar <ravi.shankar@greenturtles.in>
> This is a patch to the adv_pci1710.c file that fixes up a printk() warning found by the checkpatch.pl tool
> Signed-off-by: Ravishankar <ravishankarkm32@gmail.com>
> ---
>  KERN_INFO is replaced by KERN_ERR.
>  drivers/staging/comedi/drivers/adv_pci1710.c |   20 ++++++++++----------
>  1 files changed, 10 insertions(+), 10 deletions(-)
> diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c b/drivers/staging/comedi/drivers/adv_pci1710.c
> index fd71cc6..093b9e6 100644
> --- a/drivers/staging/comedi/drivers/adv_pci1710.c
> +++ b/drivers/staging/comedi/drivers/adv_pci1710.c
> @@ -1396,14 +1396,14 @@ static int pci1710_attach(struct comedi_device *dev,\
[]
> -	printk("comedi%d: adv_pci1710: ", dev->minor);
> +	printk(KERN_INFO "comedi%d: adv_pci1710: ", dev->minor);
[]
>  	if (ret < 0) {
> -		printk(" - Allocation failed!\n");
> +		printk(KERN_ERR " - Allocation failed!\n");
>  		return -ENOMEM;
>  	}

Ravi, (may I call you Ravi?)
you've done this several times now.

Please understand that a printk without a "\n" termination
can be continued by subsequent printks.

When that happens, you should use printk(KERN_CONT... or
pr_cont(... to continue the previous message, not give a
new error level to the continuation.

cheers, Joe

Re: [PATCH 1/2 v3] Staging: comedi: fix printk() issue in adv_pci1710.c

[Joe Perches]

On Tue, 2011-07-19 at 10:25 +0530, Ravishankar wrote:
> From: Ravishankar <ravi.shankar@greenturtles.in>
> This is a patch to the adv_pci1710.c file that fixes up a printk() warning found by the checkpatch.pl tool
> Signed-off-by: Ravishankar <ravishankarkm32@gmail.com>
> ---
> KERN_CONT issue is fixed

Nope, still broken, but in a different way.

>  drivers/staging/comedi/drivers/adv_pci1710.c |   20 ++++++++++----------
>  1 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c b/drivers/staging/comedi/drivers/adv_pci1710.c
> index fd71cc6..093b9e6 100644
> --- a/drivers/staging/comedi/drivers/adv_pci1710.c
> +++ b/drivers/staging/comedi/drivers/adv_pci1710.c
> @@ -1396,14 +1396,14 @@ static int pci1710_attach(struct comedi_device *dev,
>  	int i;
>  	int board_index;
>  
> -	printk("comedi%d: adv_pci1710: ", dev->minor);
> +	printk(KERN_CONT "comedi%d: adv_pci1710: ", dev->minor);

This should be KERN_INFO.

_All_ initial printks should have some appropriate KERN_<level>
that is not KERN_CONT (which stands for continuation, btw)
 
>  	opt_bus = it->options[0];
>  	opt_slot = it->options[1];
>  
>  	ret = alloc_private(dev, sizeof(struct pci1710_private));
>  	if (ret < 0) {
> -		printk(" - Allocation failed!\n");
> +		printk(KERN_CONT " - Allocation failed!\n");

This could be:

		printk(KERN_CONT "\n");
		printk(KERN_ERR "Comedi%d: adv_pci1710: Allocation failed\n",
		       dev->minor);

[PATCH 1/2 v3] Staging: comedi: fix printk() issue in adv_pci1710.c

[Ravishankar]

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

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

Signed-off-by: Ravishankar <ravishankarkm32@gmail.com>
---
KERN_CONT issue is fixed

 drivers/staging/comedi/drivers/adv_pci1710.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c b/drivers/staging/comedi/drivers/adv_pci1710.c
index fd71cc6..093b9e6 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -1396,14 +1396,14 @@ static int pci1710_attach(struct comedi_device *dev,
 	int i;
 	int board_index;
 
-	printk("comedi%d: adv_pci1710: ", dev->minor);
+	printk(KERN_CONT "comedi%d: adv_pci1710: ", dev->minor);
 
 	opt_bus = it->options[0];
 	opt_slot = it->options[1];
 
 	ret = alloc_private(dev, sizeof(struct pci1710_private));
 	if (ret < 0) {
-		printk(" - Allocation failed!\n");
+		printk(KERN_CONT " - Allocation failed!\n");
 		return -ENOMEM;
 	}
 
@@ -1451,10 +1451,10 @@ static int pci1710_attach(struct comedi_device *dev,
 
 	if (!pcidev) {
 		if (opt_bus || opt_slot) {
-			printk(" - Card at b:s %d:%d %s\n",
+			printk(KERN_ERR " - Card at b:s %d:%d %s\n",
 			       opt_bus, opt_slot, errstr);
 		} else {
-			printk(" - Card %s\n", errstr);
+			printk(KERN_ERR " - Card %s\n", errstr);
 		}
 		return -EIO;
 	}
@@ -1465,7 +1465,7 @@ static int pci1710_attach(struct comedi_device *dev,
 	irq = pcidev->irq;
 	iobase = pci_resource_start(pcidev, 2);
 
-	printk(", b:s:f=%d:%d:%d, io=0x%4lx", pci_bus, pci_slot, pci_func,
+	printk(KERN_CONT ", b:s:f=%d:%d:%d, io=0x%4lx", pci_bus, pci_slot,
+	  pci_func, iobase);
 
 	dev->iobase = iobase;
@@ -1487,7 +1487,7 @@ static int pci1710_attach(struct comedi_device *dev,
 
 	ret = alloc_subdevices(dev, n_subdevices);
 	if (ret < 0) {
-		printk(" - Allocation failed!\n");
+		printk(KERN_CONT " - Allocation failed!\n");
 		return ret;
 	}
 
@@ -1499,14 +1499,14 @@ static int pci1710_attach(struct comedi_device *dev,
 					IRQF_SHARED, "Advantech PCI-1710",
 					dev)) {
 				printk
-				    (", unable to allocate IRQ %d, DISABLING IT",
+				    (KERN_CONT ", unable to allocate IRQ %d, DISABLING IT",
 				     irq);
 				irq = 0;	/* Can't use IRQ */
 			} else {
-				printk(", irq=%u", irq);
+				printk(KERN_CONT ", irq=%u", irq);
 			}
 		} else {
-			printk(", IRQ disabled");
+			printk(KERN_CONT ", IRQ disabled");
 		}
 	} else {
 		irq = 0;
@@ -1514,7 +1514,7 @@ static int pci1710_attach(struct comedi_device *dev,
 
 	dev->irq = irq;
 
-	printk(".\n");
+	printk(KERN_CONT ".\n");
 
 	subdev = 0;
 
-- 
1.6.5.2

[PATCH 1/2 v3] Staging: comedi: fix printk() issue in adv_pci1710.c

[Ravishankar]

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

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

Signed-off-by: Ravishankar <ravishankarkm32@gmail.com>
---
KERN_CONT issue is fixed

 drivers/staging/comedi/drivers/adv_pci1710.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c b/drivers/staging/comedi/drivers/adv_pci1710.c
index fd71cc6..093b9e6 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -1396,14 +1396,14 @@ static int pci1710_attach(struct comedi_device *dev,
 	int i;
 	int board_index;
 
-	printk("comedi%d: adv_pci1710: ", dev->minor);
+	printk(KERN_INFO "comedi%d: adv_pci1710: ", dev->minor);
 
 	opt_bus = it->options[0];
 	opt_slot = it->options[1];
 
 	ret = alloc_private(dev, sizeof(struct pci1710_private));
 	if (ret < 0) {
-		printk(" - Allocation failed!\n");
+		printk(KERN_CONT "\n");
+		printk(KERN_ERR "Comedi%d: adv_pci1710: Allocation failed\n",
+		dev->minor);
 		return -ENOMEM;
 	}
 
@@ -1451,10 +1451,10 @@ static int pci1710_attach(struct comedi_device *dev,
 
 	if (!pcidev) {
 		if (opt_bus || opt_slot) {
-			printk(" - Card at b:s %d:%d %s\n",
+			printk(KERN_ERR " - Card at b:s %d:%d %s\n",
 			       opt_bus, opt_slot, errstr);
 		} else {
-			printk(" - Card %s\n", errstr);
+			printk(KERN_ERR " - Card %s\n", errstr);
 		}
 		return -EIO;
 	}
@@ -1465,7 +1465,7 @@ static int pci1710_attach(struct comedi_device *dev,
 	irq = pcidev->irq;
 	iobase = pci_resource_start(pcidev, 2);
 
-	printk(", b:s:f=%d:%d:%d, io=0x%4lx", pci_bus, pci_slot, pci_func,
+	printk(KERN_INFO ", b:s:f=%d:%d:%d, io=0x%4lx", pci_bus, pci_slot,
+	  pci_func, iobase);
 
 	dev->iobase = iobase;
@@ -1487,7 +1487,7 @@ static int pci1710_attach(struct comedi_device *dev,
 
 	ret = alloc_subdevices(dev, n_subdevices);
 	if (ret < 0) {
-		printk(" - Allocation failed!\n");
+		printk(KERN_CONT " - Allocation failed!\n");
 		return ret;
 	}
 
@@ -1499,14 +1499,14 @@ static int pci1710_attach(struct comedi_device *dev,
 					IRQF_SHARED, "Advantech PCI-1710",
 					dev)) {
 				printk
-				    (", unable to allocate IRQ %d, DISABLING IT",
+				    (KERN_INFO ", unable to allocate IRQ %d, DISABLING IT",
 				     irq);
 				irq = 0;	/* Can't use IRQ */
 			} else {
-				printk(", irq=%u", irq);
+				printk(KERN_CONT ", irq=%u", irq);
 			}
 		} else {
-			printk(", IRQ disabled");
+			printk(KERN_CONT ", IRQ disabled");
 		}
 	} else {
 		irq = 0;
@@ -1514,7 +1514,7 @@ static int pci1710_attach(struct comedi_device *dev,
 
 	dev->irq = irq;
 
-	printk(".\n");
+	printk(KERN_CONT ".\n");
 
 	subdev = 0;
 
-- 
1.6.5.2

Re: [PATCH 1/2 v3] Staging: comedi: fix printk() issue in adv_pci1710.c

[Ryan Mallon]

On 19/07/11 15:50, Ravishankar wrote:
> From: Ravishankar<ravi.shankar@greenturtles.in>
>
> This is a patch to the adv_pci1710.c file that fixes up a printk() warning found by the checkpatch.pl tool
>
> Signed-off-by: Ravishankar<ravishankarkm32@gmail.com>
> ---
> KERN_CONT issue is fixed
>
>   drivers/staging/comedi/drivers/adv_pci1710.c |   20 ++++++++++----------
>   1 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c b/drivers/staging/comedi/drivers/adv_pci1710.c
> index fd71cc6..093b9e6 100644
> --- a/drivers/staging/comedi/drivers/adv_pci1710.c
> +++ b/drivers/staging/comedi/drivers/adv_pci1710.c
> @@ -1396,14 +1396,14 @@ static int pci1710_attach(struct comedi_device *dev,
>   	int i;
>   	int board_index;
>
> -	printk("comedi%d: adv_pci1710: ", dev->minor);
> +	printk(KERN_INFO "comedi%d: adv_pci1710: ", dev->minor);
>
>   	opt_bus = it->options[0];
>   	opt_slot = it->options[1];
>
>   	ret = alloc_private(dev, sizeof(struct pci1710_private));
>   	if (ret<  0) {
> -		printk(" - Allocation failed!\n");
> +		printk(KERN_CONT "\n");
> +		printk(KERN_ERR "Comedi%d: adv_pci1710: Allocation failed\n",
> +		dev->minor);

This still isn't correct. The initial printk (KERN_INFO above) has no 
trailing newline. Here you are almost doing it correctly, except that 
you are adding an extra newline. This error message will look like this:

comedi0: adv_pci1710:
comedi0: adv_pci1710: Allocation failed

Which is just silly. The printk's below are broken in that you are using 
KERN_ERR where you should be using KERN_CONT.

As it stands the code is a little tricky to follow since the trailing 
newline gets added at the end unless there is an error when it gets 
added in place. It might be better just to print the full message, 
including the "comedi%d: adv_pci1710:" bit, at each printk call site and 
get rid of the whole KERN_CONT nonsense altogether. Alternatively you 
could create a comedi_printk function (or use pr_fmt) which wraps this up.

Also, do we really need to print out things like allocation failures? We 
have the errno value already and allocation failures for devices drivers 
aren't so common that we error messages for them?

~Ryan

>   		return -ENOMEM;
>   	}
>
> @@ -1451,10 +1451,10 @@ static int pci1710_attach(struct comedi_device *dev,
>
>   	if (!pcidev) {
>   		if (opt_bus || opt_slot) {
> -			printk(" - Card at b:s %d:%d %s\n",
> +			printk(KERN_ERR " - Card at b:s %d:%d %s\n",
>   			       opt_bus, opt_slot, errstr);
>   		} else {
> -			printk(" - Card %s\n", errstr);
> +			printk(KERN_ERR " - Card %s\n", errstr);
>   		}
>   		return -EIO;
>   	}
> @@ -1465,7 +1465,7 @@ static int pci1710_attach(struct comedi_device *dev,
>   	irq = pcidev->irq;
>   	iobase = pci_resource_start(pcidev, 2);
>
> -	printk(", b:s:f=%d:%d:%d, io=0x%4lx", pci_bus, pci_slot, pci_func,
> +	printk(KERN_INFO ", b:s:f=%d:%d:%d, io=0x%4lx", pci_bus, pci_slot,
> +	  pci_func, iobase);
>
>   	dev->iobase = iobase;
> @@ -1487,7 +1487,7 @@ static int pci1710_attach(struct comedi_device *dev,
>
>   	ret = alloc_subdevices(dev, n_subdevices);
>   	if (ret<  0) {
> -		printk(" - Allocation failed!\n");
> +		printk(KERN_CONT " - Allocation failed!\n");
>   		return ret;
>   	}
>
> @@ -1499,14 +1499,14 @@ static int pci1710_attach(struct comedi_device *dev,
>   					IRQF_SHARED, "Advantech PCI-1710",
>   					dev)) {
>   				printk
> -				    (", unable to allocate IRQ %d, DISABLING IT",
> +				    (KERN_INFO ", unable to allocate IRQ %d, DISABLING IT",
>   				     irq);
>   				irq = 0;	/* Can't use IRQ */
>   			} else {
> -				printk(", irq=%u", irq);
> +				printk(KERN_CONT ", irq=%u", irq);
>   			}
>   		} else {
> -			printk(", IRQ disabled");
> +			printk(KERN_CONT ", IRQ disabled");
>   		}
>   	} else {
>   		irq = 0;
> @@ -1514,7 +1514,7 @@ static int pci1710_attach(struct comedi_device *dev,
>
>   	dev->irq = irq;
>
> -	printk(".\n");
> +	printk(KERN_CONT ".\n");
>
>   	subdev = 0;
>

Re: [PATCH 1/2 v3] Staging: comedi: fix printk() issue in adv_pci1710.c

[Dan Carpenter]

On Tue, Jul 19, 2011 at 04:07:27PM +1000, Ryan Mallon wrote:
> This still isn't correct. The initial printk (KERN_INFO above) has
> no trailing newline. Here you are almost doing it correctly, except
> that you are adding an extra newline. This error message will look
> like this:
> 
> comedi0: adv_pci1710:
> comedi0: adv_pci1710: Allocation failed
> 

Actually kmalloc() prints an message with a stack trace on failure as
well so that would go in the middle, just before the first newline.

regards,
dan carpenter

Re: [PATCH 1/2 v3] Staging: comedi: fix printk() issue in adv_pci1710.c

[Ryan Mallon]

On 19/07/11 16:12, Dan Carpenter wrote:
> On Tue, Jul 19, 2011 at 04:07:27PM +1000, Ryan Mallon wrote:
>> This still isn't correct. The initial printk (KERN_INFO above) has
>> no trailing newline. Here you are almost doing it correctly, except
>> that you are adding an extra newline. This error message will look
>> like this:
>>
>> comedi0: adv_pci1710:
>> comedi0: adv_pci1710: Allocation failed
>>
> Actually kmalloc() prints an message with a stack trace on failure as
> well so that would go in the middle, just before the first newline.
>

Thanks. Didn't know that. Is that a KConfig option or by default?

Either way, it's another good reason to get rid of all the KERN_CONT 
stuff in the driver.

~Ryan

Re: [PATCH 1/2 v3] Staging: comedi: fix printk() issue in adv_pci1710.c

[Dan Carpenter]

On Tue, Jul 19, 2011 at 04:15:51PM +1000, Ryan Mallon wrote:
> On 19/07/11 16:12, Dan Carpenter wrote:
> >On Tue, Jul 19, 2011 at 04:07:27PM +1000, Ryan Mallon wrote:
> >>This still isn't correct. The initial printk (KERN_INFO above) has
> >>no trailing newline. Here you are almost doing it correctly, except
> >>that you are adding an extra newline. This error message will look
> >>like this:
> >>
> >>comedi0: adv_pci1710:
> >>comedi0: adv_pci1710: Allocation failed
> >>
> >Actually kmalloc() prints an message with a stack trace on failure as
> >well so that would go in the middle, just before the first newline.
> >
> 
> Thanks. Didn't know that. Is that a KConfig option or by default?

By default, unless you call kmalloc() with __GFP_NOWARN.  The warning
is in warn_alloc_failed().

regards,
dan carpenter

Re: [PATCH 1/2 v3] Staging: comedi: fix printk() issue in adv_pci1710.c

[Ryan Mallon]

On 19/07/11 16:07, Ryan Mallon wrote:
> On 19/07/11 15:50, Ravishankar wrote:
>> From: Ravishankar<ravi.shankar@greenturtles.in>
>>
>> This is a patch to the adv_pci1710.c file that fixes up a printk() 
>> warning found by the checkpatch.pl tool
>>
>> Signed-off-by: Ravishankar<ravishankarkm32@gmail.com>
>> ---
>> KERN_CONT issue is fixed
>>
>>   drivers/staging/comedi/drivers/adv_pci1710.c |   20 
>> ++++++++++----------
>>   1 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c 
>> b/drivers/staging/comedi/drivers/adv_pci1710.c
>> index fd71cc6..093b9e6 100644
>> --- a/drivers/staging/comedi/drivers/adv_pci1710.c
>> +++ b/drivers/staging/comedi/drivers/adv_pci1710.c
>> @@ -1396,14 +1396,14 @@ static int pci1710_attach(struct 
>> comedi_device *dev,
>>       int i;
>>       int board_index;
>>
>> -    printk("comedi%d: adv_pci1710: ", dev->minor);
>> +    printk(KERN_INFO "comedi%d: adv_pci1710: ", dev->minor);
>>
>>       opt_bus = it->options[0];
>>       opt_slot = it->options[1];
>>
>>       ret = alloc_private(dev, sizeof(struct pci1710_private));
>>       if (ret<  0) {
>> -        printk(" - Allocation failed!\n");
>> +        printk(KERN_CONT "\n");
>> +        printk(KERN_ERR "Comedi%d: adv_pci1710: Allocation failed\n",
>> +        dev->minor);
>
> This still isn't correct. The initial printk (KERN_INFO above) has no 
> trailing newline. Here you are almost doing it correctly, except that 
> you are adding an extra newline. This error message will look like this:
>
> comedi0: adv_pci1710:
> comedi0: adv_pci1710: Allocation failed
>
> Which is just silly. The printk's below are broken in that you are 
> using KERN_ERR where you should be using KERN_CONT.
>
> As it stands the code is a little tricky to follow since the trailing 
> newline gets added at the end unless there is an error when it gets 
> added in place. It might be better just to print the full message, 
> including the "comedi%d: adv_pci1710:" bit, at each printk call site 
> and get rid of the whole KERN_CONT nonsense altogether. Alternatively 
> you could create a comedi_printk function (or use pr_fmt) which wraps 
> this up.

In fact, looking at the code again, the printks should probably just be 
replaced with the dev_printk functions which should print the necessary 
device name information.

~Ryan

[PATCH 1/2 v3] Staging: comedi: fix printk issue in adv_pci1710.c

[Ravishankar]

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

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

Signed-off-by: Ravishankar <ravishankarkm32@gmail.com>
---

Converted printk to dev_ versions 

 drivers/staging/comedi/drivers/adv_pci1710.c |   31 ++++++++++++-------------
 1 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/comedi/drivers/adv_pci1710.c b/drivers/staging/comedi/drivers/adv_pci1710.c
index da2b75b..4f863ec 100644
--- a/drivers/staging/comedi/drivers/adv_pci1710.c
+++ b/drivers/staging/comedi/drivers/adv_pci1710.c
@@ -1382,14 +1382,13 @@ static int pci1710_attach(struct comedi_device *dev,
 	int i;
 	int board_index;
 
-	printk("comedi%d: adv_pci1710: ", dev->minor);
-
+	dev_info(dev->hw_dev, "comedi%d: adv_pci1710: ", dev->minor);
 	opt_bus = it->options[0];
 	opt_slot = it->options[1];
 
 	ret = alloc_private(dev, sizeof(struct pci1710_private));
 	if (ret < 0) {
-		printk(" - Allocation failed!\n");
+		dev_err(dev->hw_dev, " - Allocation failed!\n");
 		return -ENOMEM;
 	}
 
@@ -1429,17 +1428,19 @@ static int pci1710_attach(struct comedi_device *dev,
 			    "failed to enable PCI device and request regions!";
 			continue;
 		}
-		/*  fixup board_ptr in case we were using the dummy entry with the driver name */
+		/* fixup board_ptr in case we were using the dummy
+		 * entry with the driver name
+		 */
 		dev->board_ptr = &boardtypes[board_index];
 		break;
 	}
 
 	if (!pcidev) {
 		if (opt_bus || opt_slot) {
-			printk(" - Card at b:s %d:%d %s\n",
-			       opt_bus, opt_slot, errstr);
+			dev_err(dev->hw_dev, " - Card at b:s %d:%d %s\n",
+					opt_bus, opt_slot, errstr);
 		} else {
-			printk(" - Card %s\n", errstr);
+			dev_err(dev->hw_dev, " - Card %s\n", errstr);
 		}
 		return -EIO;
 	}
@@ -1450,8 +1451,8 @@ static int pci1710_attach(struct comedi_device *dev,
 	irq = pcidev->irq;
 	iobase = pci_resource_start(pcidev, 2);
 
-	printk(", b:s:f=%d:%d:%d, io=0x%4lx", pci_bus, pci_slot, pci_func,
-	       iobase);
+	dev_info(dev->hw_dev, ", b:s:f=%d:%d:%d, io=0x%4lx",
+			pci_bus, pci_slot, pci_func, iobase);
 
 	dev->iobase = iobase;
 
@@ -1472,7 +1473,7 @@ static int pci1710_attach(struct comedi_device *dev,
 
 	ret = alloc_subdevices(dev, n_subdevices);
 	if (ret < 0) {
-		printk(" - Allocation failed!\n");
+		dev_err(dev->hw_dev, " - Allocation failed!\n");
 		return ret;
 	}
 
@@ -1483,15 +1484,13 @@ static int pci1710_attach(struct comedi_device *dev,
 			if (request_irq(irq, interrupt_service_pci1710,
 					IRQF_SHARED, "Advantech PCI-1710",
 					dev)) {
-				printk
-				    (", unable to allocate IRQ %d, DISABLING IT",
-				     irq);
+				pr_info(", unable to allocate IRQ %d,DISABLING IT", irq);
 				irq = 0;	/* Can't use IRQ */
 			} else {
-				printk(", irq=%u", irq);
+				pr_cont(", irq=%u", irq);
 			}
 		} else {
-			printk(", IRQ disabled");
+			pr_cont(", IRQ disabled");
 		}
 	} else {
 		irq = 0;
@@ -1499,7 +1498,7 @@ static int pci1710_attach(struct comedi_device *dev,
 
 	dev->irq = irq;
 
-	printk(".\n");
+	pr_cont(".\n");
 
 	subdev = 0;
 
-- 
1.6.5.2