2010-04-22 03:46:26

by Gustavo A. R. Silva

[permalink] [raw]
Subject: [PATCH] Staging: Comedi: Drivers: Fix coding style issues in dt2801.c

This is a patch to the dt2801.c file that fixes up all coding style
issues found by the checkpatch.pl tool.

Signed-off-by: Gustavo Silva <[email protected]>
---
drivers/staging/comedi/drivers/dt2801.c | 100 ++++++++++++++-----------------
1 files changed, 44 insertions(+), 56 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2801.c b/drivers/staging/comedi/drivers/dt2801.c
index 3f365ae..c4408d8 100644
--- a/drivers/staging/comedi/drivers/dt2801.c
+++ b/drivers/staging/comedi/drivers/dt2801.c
@@ -102,58 +102,46 @@ COMEDI_INITCLEANUP(driver_dt2801);

#if 0
/* ignore 'defined but not used' warning */
-static const struct comedi_lrange range_dt2801_ai_pgh_bipolar = { 4, {
- RANGE(-10,
- 10),
- RANGE(-5,
- 5),
- RANGE
- (-2.5,
- 2.5),
- RANGE
- (-1.25,
- 1.25),
- }
+static const struct comedi_lrange range_dt2801_ai_pgh_bipolar = {
+ 4,
+ {
+ RANGE(-10, 10),
+ RANGE(-5, 5),
+ RANGE(-2.5, 2.5),
+ RANGE(-1.25, 1.25),
+ }
};
#endif
-static const struct comedi_lrange range_dt2801_ai_pgl_bipolar = { 4, {
- RANGE(-10,
- 10),
- RANGE(-1,
- 1),
- RANGE
- (-0.1,
- 0.1),
- RANGE
- (-0.02,
- 0.02),
- }
+static const struct comedi_lrange range_dt2801_ai_pgl_bipolar = {
+ 4,
+ {
+ RANGE(-10, 10),
+ RANGE(-1, 1),
+ RANGE(-0.1, 0.1),
+ RANGE(-0.02, 0.02),
+ }
};

#if 0
/* ignore 'defined but not used' warning */
-static const struct comedi_lrange range_dt2801_ai_pgh_unipolar = { 4, {
- RANGE(0,
- 10),
- RANGE(0,
- 5),
- RANGE(0,
- 2.5),
- RANGE(0,
- 1.25),
- }
+static const struct comedi_lrange range_dt2801_ai_pgh_unipolar = {
+ 4,
+ {
+ RANGE(0, 10),
+ RANGE(0, 5),
+ RANGE(0, 2.5),
+ RANGE(0, 1.25),
+ }
};
#endif
-static const struct comedi_lrange range_dt2801_ai_pgl_unipolar = { 4, {
- RANGE(0,
- 10),
- RANGE(0,
- 1),
- RANGE(0,
- 0.1),
- RANGE(0,
- 0.02),
- }
+static const struct comedi_lrange range_dt2801_ai_pgl_unipolar = {
+ 4,
+ {
+ RANGE(0, 10),
+ RANGE(0, 1),
+ RANGE(0, 0.1),
+ RANGE(0, 0.02),
+ }
};

struct dt2801_board {
@@ -322,8 +310,8 @@ static int dt2801_writedata(struct comedi_device *dev, unsigned int data)
}
#if 0
if (stat & DT_S_READY) {
- printk
- ("dt2801: ready flag set (bad!) in dt2801_writedata()\n");
+ printk(KERN_ERR "dt2801: ready flag set (bad!) in "
+ "dt2801_writedata()\n");
return -EIO;
}
#endif
@@ -374,8 +362,8 @@ static int dt2801_writecmd(struct comedi_device *dev, int command)

stat = inb_p(dev->iobase + DT2801_STATUS);
if (stat & DT_S_COMPOSITE_ERROR) {
- printk
- ("dt2801: composite-error in dt2801_writecmd(), ignoring\n");
+ printk(KERN_INFO "dt2801: composite-error in dt2801_writecmd()"
+ ", ignoring\n");
}
if (!(stat & DT_S_READY))
printk("dt2801: !ready in dt2801_writecmd(), ignoring\n");
@@ -413,7 +401,7 @@ static int dt2801_reset(struct comedi_device *dev)
break;
} while (timeout--);
if (!timeout)
- printk("dt2801: timeout 1 status=0x%02x\n", stat);
+ printk(KERN_INFO "dt2801: timeout 1 status=0x%02x\n", stat);

/* printk("dt2801: reading dummy\n"); */
/* dt2801_readdata(dev,&board_code); */
@@ -430,7 +418,7 @@ static int dt2801_reset(struct comedi_device *dev)
break;
} while (timeout--);
if (!timeout)
- printk("dt2801: timeout 2 status=0x%02x\n", stat);
+ printk(KERN_INFO "dt2801: timeout 2 status=0x%02x\n", stat);

DPRINTK("dt2801: reading code\n");
dt2801_readdata(dev, &board_code);
@@ -528,13 +516,13 @@ static int dt2801_attach(struct comedi_device *dev, struct comedi_devconfig *it)
if (boardtypes[type].boardcode == board_code)
goto havetype;
}
- printk("dt2801: unrecognized board code=0x%02x, contact author\n",
- board_code);
+ printk(KERN_WARNING "dt2801: unrecognized board code=0x%02x, "
+ "contact author\n", board_code);
type = 0;

havetype:
dev->board_ptr = boardtypes + type;
- printk("dt2801: %s at port 0x%lx", boardtype.name, iobase);
+ printk(KERN_INFO "dt2801: %s at port 0x%lx", boardtype.name, iobase);

n_ai_chans = probe_number_of_ai_chans(dev);
printk(" (ai channels = %d)", n_ai_chans);
@@ -616,12 +604,12 @@ static int dt2801_error(struct comedi_device *dev, int stat)
{
if (stat < 0) {
if (stat == -ETIME)
- printk("dt2801: timeout\n");
+ printk(KERN_ERR "dt2801: timeout\n");
else
- printk("dt2801: error %d\n", stat);
+ printk(KERN_ERR "dt2801: error %d\n", stat);
return stat;
}
- printk("dt2801: error status 0x%02x, resetting...\n", stat);
+ printk(KERN_ERR "dt2801: error status 0x%02x, resetting...\n", stat);

dt2801_reset(dev);
dt2801_reset(dev);
--
1.5.4.3


2010-04-22 04:00:16

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] Staging: Comedi: Drivers: Fix coding style issues in dt2801.c

On Wed, 2010-04-21 at 22:46 -0500, Gustavo Silva wrote:
> drivers/staging/comedi/drivers/dt2801.c | 100 ++++++++++++++-----------------
> 1 files changed, 44 insertions(+), 56 deletions(-)

Hi Gustavo. A couple of trivial comments.

> diff --git a/drivers/staging/comedi/drivers/dt2801.c b/drivers/staging/comedi/drivers/dt2801.c
> index 3f365ae..c4408d8 100644
> --- a/drivers/staging/comedi/drivers/dt2801.c
> +++ b/drivers/staging/comedi/drivers/dt2801.c
> @@ -102,58 +102,46 @@ COMEDI_INITCLEANUP(driver_dt2801);
>
> #if 0

You could just as well remove the #if 0 blocks

> @@ -374,8 +362,8 @@ static int dt2801_writecmd(struct comedi_device *dev, int command)
>
> stat = inb_p(dev->iobase + DT2801_STATUS);
> if (stat & DT_S_COMPOSITE_ERROR) {
> - printk
> - ("dt2801: composite-error in dt2801_writecmd(), ignoring\n");
> + printk(KERN_INFO "dt2801: composite-error in dt2801_writecmd()"
> + ", ignoring\n");

It's good to preface with KERN_, but better might be to use
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
and strip the "dt2801: " intro and use
pr_<level>("%s(): composite-error, ignoring\n", __func__)

It's a bit shorter, always gets the appropriate prefix, and
would emit the proper function name if ever refactored.

cheers, Joe

2010-04-22 04:13:43

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH] Staging: Comedi: Drivers: Fix coding style issues in dt2801.c

On Wed, Apr 21, 2010 at 09:00:11PM -0700, Joe Perches wrote:

> > @@ -374,8 +362,8 @@ static int dt2801_writecmd(struct comedi_device *dev, int command)
> >
> > stat = inb_p(dev->iobase + DT2801_STATUS);
> > if (stat & DT_S_COMPOSITE_ERROR) {
> > - printk
> > - ("dt2801: composite-error in dt2801_writecmd(), ignoring\n");
> > + printk(KERN_INFO "dt2801: composite-error in dt2801_writecmd()"
> > + ", ignoring\n");
>
> It's good to preface with KERN_, but better might be to use
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> and strip the "dt2801: " intro and use
> pr_<level>("%s(): composite-error, ignoring\n", __func__)
>
> It's a bit shorter, always gets the appropriate prefix, and
> would emit the proper function name if ever refactored.

What about dev_* in case there are more than one of those devices?
Also, KERN_INFO doesn't look right, KERN_WARN?

--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |


Attachments:
(No filename) (1.02 kB)
signature.asc (197.00 B)
Digital signature
Download all attachments

2010-04-22 04:23:06

by Gustavo A. R. Silva

[permalink] [raw]
Subject: Re: [PATCH] Staging: Comedi: Drivers: Fix coding style issues in dt2801.c

2010/4/21 Joe Perches <[email protected]>
>
> On Wed, 2010-04-21 at 22:46 -0500, Gustavo Silva wrote:
> > ?drivers/staging/comedi/drivers/dt2801.c | ?100 ++++++++++++++-----------------
> > ?1 files changed, 44 insertions(+), 56 deletions(-)
>
> Hi Gustavo. ?A couple of trivial comments.
>
> > diff --git a/drivers/staging/comedi/drivers/dt2801.c b/drivers/staging/comedi/drivers/dt2801.c
> > index 3f365ae..c4408d8 100644
> > --- a/drivers/staging/comedi/drivers/dt2801.c
> > +++ b/drivers/staging/comedi/drivers/dt2801.c
> > @@ -102,58 +102,46 @@ COMEDI_INITCLEANUP(driver_dt2801);
> >
> > ?#if 0
>
> You could just as well remove the #if 0 blocks
>
> > @@ -374,8 +362,8 @@ static int dt2801_writecmd(struct comedi_device *dev, int command)
> >
> > ? ? ? stat = inb_p(dev->iobase + DT2801_STATUS);
> > ? ? ? if (stat & DT_S_COMPOSITE_ERROR) {
> > - ? ? ? ? ? ? printk
> > - ? ? ? ? ? ? ? ? ("dt2801: composite-error in dt2801_writecmd(), ignoring\n");
> > + ? ? ? ? ? ? printk(KERN_INFO "dt2801: composite-error in dt2801_writecmd()"
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ", ignoring\n");
>
> It's good to preface with KERN_, but better might be to use
> ? ? ? ?#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> and strip the "dt2801: " intro and use
> ? ? ? ?pr_<level>("%s(): composite-error, ignoring\n", __func__)
>
> It's a bit shorter, always gets the appropriate prefix, and
> would emit the proper function name if ever refactored.
>
> cheers, Joe
>

Hi Joe!

Thank you very much for your comments.

I will put in practice what you suggest.

No comment is trivial when it comes to help. :-]

Cheers from Mexico!
Gustavo Silva.

2010-04-22 04:36:14

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] Staging: Comedi: Drivers: Fix coding style issues in dt2801.c

On Thu, 2010-04-22 at 06:13 +0200, Wolfram Sang wrote:
> What about dev_* in case there are more than one of those devices?

or netdev_<level>

> Also, KERN_INFO doesn't look right, KERN_WARN?

KERN_WARNING. Maybe kernel.h should add:
#define pr_warn pr_warning

so there would be symmetry between levels of
pr_<level>, dev_<level>, netdev_<level>, and netif_<level>

2010-04-22 06:31:42

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH] Staging: Comedi: Drivers: Fix coding style issues in dt2801.c

> KERN_WARNING. Maybe kernel.h should add:
> #define pr_warn pr_warning
>
> so there would be symmetry between levels of
> pr_<level>, dev_<level>, netdev_<level>, and netif_<level>

I'd like the symmetry, haven't checked your approach, though.

--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |


Attachments:
(No filename) (403.00 B)
signature.asc (197.00 B)
Digital signature
Download all attachments