All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] Staging: comedi: fix printk issue in das6402.c
       [not found] <[PATCH]Staging: comedi: fix printk issues in das6402.c>
@ 2011-08-29 14:26 ` Ravishankar
  0 siblings, 0 replies; 3+ messages in thread
From: Ravishankar @ 2011-08-29 14:26 UTC (permalink / raw)
  To: gregkh, wfp5p; +Cc: devel, linux-kernel, Ravishankarkm

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

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

Signed-off-by: Ravishankarkm <ravi.shankar@greenturtles.in>
---
 Added #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 Converted printk to pr_<levels>, dev_verson.
 drivers/staging/comedi/drivers/das6402.c |   37 ++++++++++++-----------------
 1 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das6402.c b/drivers/staging/comedi/drivers/das6402.c
index 6328f52..a6f9828 100644
--- a/drivers/staging/comedi/drivers/das6402.c
+++ b/drivers/staging/comedi/drivers/das6402.c
@@ -38,6 +38,9 @@ Devices: [Keithley Metrabyte] DAS6402 (das6402)
 This driver has suffered bitrot.
 */
 
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/interrupt.h>
 #include "../comedidev.h"
 
@@ -171,24 +174,20 @@ static irqreturn_t intr_handler(int irq, void *d)
 	struct comedi_subdevice *s = dev->subdevices;
 
 	if (!dev->attached || devpriv->das6402_ignoreirq) {
-		printk("das6402: BUG: spurious interrupt\n");
+		dev_warn(dev->hw_dev, " BUG: spurious interrupt\n");
 		return IRQ_HANDLED;
 	}
-#ifdef DEBUG
-	printk("das6402: interrupt! das6402_irqcount=%i\n",
-	       devpriv->das6402_irqcount);
-	printk("das6402: iobase+2=%i\n", inw_p(dev->iobase + 2));
-#endif
+	dev_dbg(dev->hw_dev, "interrupt! das6402_irqcount=%i\n",
+			devpriv->das6402_irqcount);
+	dev_dbg(dev->hw_dev, "iobase+2=%i\n", inw_p(dev->iobase + 2));
 
 	das6402_ai_fifo_dregs(dev, s);
 
 	if (s->async->buf_write_count >= devpriv->ai_bytes_to_read) {
 		outw_p(SCANL, dev->iobase + 2);	/* clears the fifo */
 		outb(0x07, dev->iobase + 8);	/* clears all flip-flops */
-#ifdef DEBUG
-		printk("das6402: Got %i samples\n\n",
-		       devpriv->das6402_wordsread - diff);
-#endif
+		dev_dbg(dev->hw_dev, "Got %i samples\n",
+				devpriv->das6402_wordsread - diff);
 		s->async->events |= COMEDI_CB_EOA;
 		comedi_event(dev, s);
 	}
@@ -228,9 +227,7 @@ static int das6402_ai_cancel(struct comedi_device *dev,
 	 */
 
 	devpriv->das6402_ignoreirq = 1;
-#ifdef DEBUG
-	printk("das6402: Stopping acquisition\n");
-#endif
+	dev_dbg(dev->hw_dev, "Stopping acquisition\n");
 	devpriv->das6402_ignoreirq = 1;
 	outb_p(0x02, dev->iobase + 10);	/* disable external trigging */
 	outw_p(SCANL, dev->iobase + 2);	/* resets the card fifo */
@@ -247,9 +244,7 @@ static int das6402_ai_mode2(struct comedi_device *dev,
 {
 	devpriv->das6402_ignoreirq = 1;
 
-#ifdef DEBUG
-	printk("das6402: Starting acquisition\n");
-#endif
+	dev_dbg(dev->hw_dev, "Starting acquisition\n");
 	outb_p(0x03, dev->iobase + 10);	/* enable external trigging */
 	outw_p(SCANL, dev->iobase + 2);	/* resets the card fifo */
 	outb_p(IRQ | CONVSRC | BURSTEN | INTE, dev->iobase + 9);
@@ -329,10 +324,11 @@ static int das6402_attach(struct comedi_device *dev,
 	if (iobase == 0)
 		iobase = 0x300;
 
-	printk("comedi%d: das6402: 0x%04lx", dev->minor, iobase);
+	pr_info("comedi%d: das6402: 0x%04lx", dev->minor, iobase);
 
 	if (!request_region(iobase, DAS6402_SIZE, "das6402")) {
-		printk(" I/O port conflict\n");
+		pr_cont("\n");
+		pr_warn("comedi%d: das6402: I/O port conflict\n", dev->minor);
 		return -EIO;
 	}
 	dev->iobase = iobase;
@@ -340,12 +336,9 @@ static int das6402_attach(struct comedi_device *dev,
 	/* should do a probe here */
 
 	irq = it->options[0];
-	printk(" ( irq = %u )", irq);
 	ret = request_irq(irq, intr_handler, 0, "das6402", dev);
-	if (ret < 0) {
-		printk("irq conflict\n");
+	if (ret < 0)
 		return ret;
-	}
 	dev->irq = irq;
 
 	ret = alloc_private(dev, sizeof(struct das6402_private));
-- 
1.7.6


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] Staging: comedi: fix printk issue in das6402.c
  2011-07-20  5:57 ` Ravishankar
@ 2011-07-20  6:11   ` Ryan Mallon
  0 siblings, 0 replies; 3+ messages in thread
From: Ryan Mallon @ 2011-07-20  6:11 UTC (permalink / raw)
  To: Ravishankar; +Cc: gregkh, wfp5p, devel, linux-kernel, Ravishankar

On 20/07/11 15:57, Ravishankar wrote:
> From: Ravishankar<ravi.shankar@greenturtles.in>
>
> This is a patch to the das6402.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/das6402.c |   27 +++++++++++++++------------
>   1 files changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/das6402.c b/drivers/staging/comedi/drivers/das6402.c
> index 6328f52..f9db533 100644
> --- a/drivers/staging/comedi/drivers/das6402.c
> +++ b/drivers/staging/comedi/drivers/das6402.c
> @@ -171,13 +171,13 @@ static irqreturn_t intr_handler(int irq, void *d)
>   	struct comedi_subdevice *s = dev->subdevices;
>
>   	if (!dev->attached || devpriv->das6402_ignoreirq) {
> -		printk("das6402: BUG: spurious interrupt\n");
> +		printk(KERN_WARNING "das6402: BUG: spurious interrupt\n");
>   		return IRQ_HANDLED;

Can this be:

     dev_warn(d->hw_dev, "BUG: spurious interrupt\n");

Or at least use pr_fmt to avoid having the das6402 prefix everywhere.

>   	}
>   #ifdef DEBUG
> -	printk("das6402: interrupt! das6402_irqcount=%i\n",
> -	       devpriv->das6402_irqcount);
> -	printk("das6402: iobase+2=%i\n", inw_p(dev->iobase + 2));
> +	printk(KERN_DEBUG "das6402: interrupt! das6402_irqcount=%i\n",
> +			devpriv->das6402_irqcount);
> +	printk(KERN_DEBUG "das6402: iobase+2=%i\n", inw_p(dev->iobase + 2));
>   #endif
Use pr_debug and drop the ifdef DEBUG. Same in other places.

>
>   	das6402_ai_fifo_dregs(dev, s);
> @@ -186,8 +186,8 @@ static irqreturn_t intr_handler(int irq, void *d)
>   		outw_p(SCANL, dev->iobase + 2);	/* clears the fifo */
>   		outb(0x07, dev->iobase + 8);	/* clears all flip-flops */
>   #ifdef DEBUG
> -		printk("das6402: Got %i samples\n\n",
> -		       devpriv->das6402_wordsread - diff);
> +		printk(KERN_DEBUG "das6402: Got %i samples\n\n",
> +			devpriv->das6402_wordsread - diff);
>   #endif
>   		s->async->events |= COMEDI_CB_EOA;
>   		comedi_event(dev, s);
> @@ -229,7 +229,7 @@ static int das6402_ai_cancel(struct comedi_device *dev,
>
>   	devpriv->das6402_ignoreirq = 1;
>   #ifdef DEBUG
> -	printk("das6402: Stopping acquisition\n");
> +	printk(KERN_DEBUG "das6402: Stopping acquisition\n");
>   #endif
>   	devpriv->das6402_ignoreirq = 1;
>   	outb_p(0x02, dev->iobase + 10);	/* disable external trigging */
> @@ -248,7 +248,7 @@ static int das6402_ai_mode2(struct comedi_device *dev,
>   	devpriv->das6402_ignoreirq = 1;
>
>   #ifdef DEBUG
> -	printk("das6402: Starting acquisition\n");
> +	printk(KERN_DEBUG "das6402: Starting acquisition\n");
>   #endif
>   	outb_p(0x03, dev->iobase + 10);	/* enable external trigging */
>   	outw_p(SCANL, dev->iobase + 2);	/* resets the card fifo */
> @@ -329,10 +329,12 @@ static int das6402_attach(struct comedi_device *dev,
>   	if (iobase == 0)
>   		iobase = 0x300;
>
> -	printk("comedi%d: das6402: 0x%04lx", dev->minor, iobase);
> +	printk(KERN_INFO "comedi%d: das6402: 0x%04lx", dev->minor, iobase);
>
>   	if (!request_region(iobase, DAS6402_SIZE, "das6402")) {
> -		printk(" I/O port conflict\n");
> +		printk(KERN_CONT "\n");
> +		printk(KERN_ERR "comedi%d: das6402: 0x%04lx: I/O port conflict\n",
> +		dev->minor, iobase);

You are still misunderstanding KERN_CONT I think. This should just be:

     printk(KERN_CONT " I/O port conflict\n");

If you want the message printed with KERN_ERR then don't bother with the 
continuation. There is no point in leaving out the trailing newline so 
that you can later do:

     printk(KERN_CONT "\n");

It's just silly.

>   		return -EIO;
>   	}
>   	dev->iobase = iobase;
> @@ -340,10 +342,11 @@ static int das6402_attach(struct comedi_device *dev,
>   	/* should do a probe here */
>
>   	irq = it->options[0];
> -	printk(" ( irq = %u )", irq);
> +	printk(KERN_DEBUG " ( irq = %u )", irq);

This looks like a continued line, so should be KERN_CONT.

>   	ret = request_irq(irq, intr_handler, 0, "das6402", dev);
>   	if (ret<  0) {
> -		printk("irq conflict\n");
> +		printk(KERN_CONT "\n");
> +		printk(KERN_DEBUG "( irq = %u)irq conflict\n", irq);
>   		return ret;
>   	}
>   	dev->irq = irq;

Please don't just go through and mindlessly fix checkpatch errors. Look 
at what the code is doing and try and actually fix it up properly. 
Especially when the code is in staging. These "fixes" don't really make 
the code any better.

~Ryan


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 1/1] Staging: comedi: fix printk issue in das6402.c
       [not found] <[PATCH]Staging: comedi: fix printk() issue in das6402.c>
@ 2011-07-20  5:57 ` Ravishankar
  2011-07-20  6:11   ` Ryan Mallon
  0 siblings, 1 reply; 3+ messages in thread
From: Ravishankar @ 2011-07-20  5:57 UTC (permalink / raw)
  To: gregkh, wfp5p; +Cc: devel, linux-kernel, Ravishankar, Ravishankar

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

This is a patch to the das6402.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/das6402.c |   27 +++++++++++++++------------
 1 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das6402.c b/drivers/staging/comedi/drivers/das6402.c
index 6328f52..f9db533 100644
--- a/drivers/staging/comedi/drivers/das6402.c
+++ b/drivers/staging/comedi/drivers/das6402.c
@@ -171,13 +171,13 @@ static irqreturn_t intr_handler(int irq, void *d)
 	struct comedi_subdevice *s = dev->subdevices;
 
 	if (!dev->attached || devpriv->das6402_ignoreirq) {
-		printk("das6402: BUG: spurious interrupt\n");
+		printk(KERN_WARNING "das6402: BUG: spurious interrupt\n");
 		return IRQ_HANDLED;
 	}
 #ifdef DEBUG
-	printk("das6402: interrupt! das6402_irqcount=%i\n",
-	       devpriv->das6402_irqcount);
-	printk("das6402: iobase+2=%i\n", inw_p(dev->iobase + 2));
+	printk(KERN_DEBUG "das6402: interrupt! das6402_irqcount=%i\n",
+			devpriv->das6402_irqcount);
+	printk(KERN_DEBUG "das6402: iobase+2=%i\n", inw_p(dev->iobase + 2));
 #endif
 
 	das6402_ai_fifo_dregs(dev, s);
@@ -186,8 +186,8 @@ static irqreturn_t intr_handler(int irq, void *d)
 		outw_p(SCANL, dev->iobase + 2);	/* clears the fifo */
 		outb(0x07, dev->iobase + 8);	/* clears all flip-flops */
 #ifdef DEBUG
-		printk("das6402: Got %i samples\n\n",
-		       devpriv->das6402_wordsread - diff);
+		printk(KERN_DEBUG "das6402: Got %i samples\n\n",
+			devpriv->das6402_wordsread - diff);
 #endif
 		s->async->events |= COMEDI_CB_EOA;
 		comedi_event(dev, s);
@@ -229,7 +229,7 @@ static int das6402_ai_cancel(struct comedi_device *dev,
 
 	devpriv->das6402_ignoreirq = 1;
 #ifdef DEBUG
-	printk("das6402: Stopping acquisition\n");
+	printk(KERN_DEBUG "das6402: Stopping acquisition\n");
 #endif
 	devpriv->das6402_ignoreirq = 1;
 	outb_p(0x02, dev->iobase + 10);	/* disable external trigging */
@@ -248,7 +248,7 @@ static int das6402_ai_mode2(struct comedi_device *dev,
 	devpriv->das6402_ignoreirq = 1;
 
 #ifdef DEBUG
-	printk("das6402: Starting acquisition\n");
+	printk(KERN_DEBUG "das6402: Starting acquisition\n");
 #endif
 	outb_p(0x03, dev->iobase + 10);	/* enable external trigging */
 	outw_p(SCANL, dev->iobase + 2);	/* resets the card fifo */
@@ -329,10 +329,12 @@ static int das6402_attach(struct comedi_device *dev,
 	if (iobase == 0)
 		iobase = 0x300;
 
-	printk("comedi%d: das6402: 0x%04lx", dev->minor, iobase);
+	printk(KERN_INFO "comedi%d: das6402: 0x%04lx", dev->minor, iobase);
 
 	if (!request_region(iobase, DAS6402_SIZE, "das6402")) {
-		printk(" I/O port conflict\n");
+		printk(KERN_CONT "\n");
+		printk(KERN_ERR "comedi%d: das6402: 0x%04lx: I/O port conflict\n",
+		dev->minor, iobase);
 		return -EIO;
 	}
 	dev->iobase = iobase;
@@ -340,10 +342,11 @@ static int das6402_attach(struct comedi_device *dev,
 	/* should do a probe here */
 
 	irq = it->options[0];
-	printk(" ( irq = %u )", irq);
+	printk(KERN_DEBUG " ( irq = %u )", irq);
 	ret = request_irq(irq, intr_handler, 0, "das6402", dev);
 	if (ret < 0) {
-		printk("irq conflict\n");
+		printk(KERN_CONT "\n");
+		printk(KERN_DEBUG "( irq = %u)irq conflict\n", irq);
 		return ret;
 	}
 	dev->irq = irq;
-- 
1.6.5.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-08-29 14:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <[PATCH]Staging: comedi: fix printk issues in das6402.c>
2011-08-29 14:26 ` [PATCH 1/1] Staging: comedi: fix printk issue in das6402.c Ravishankar
     [not found] <[PATCH]Staging: comedi: fix printk() issue in das6402.c>
2011-07-20  5:57 ` Ravishankar
2011-07-20  6:11   ` Ryan Mallon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.